Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HER 16: Migration -- remove name column from users table #17

Merged
merged 2 commits into from
Dec 5, 2024

Conversation

trca831
Copy link
Collaborator

@trca831 trca831 commented Nov 25, 2024

[HER-16] Migration: Drop name column from User

Issue

https://raquelanaroman.atlassian.net/browse/HER-16

As a developer, I would like to remove tech debt from my app by removing the name column from users table.

Acceptance Criteria: create a migration that removes the name column from users

Links:

Changes

  • Generated and ran a migration to Remove Name from Users table.
  • Verified that change was completed by checking schema file.
  • Updated Application code by removing (commenting out) name references in app/models/User.rb, spec/factories/Users.rb, and spec/models/User-spec.rb

Review Checklist

  • I have documented my code with code comments.

Copy link
Collaborator

@raquii raquii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your migration is great! I want you to run this on your branch, though

git checkout HER-16-Migration-drop-name-column-from-Users
git fetch
git rebase origin/main
# resolve any conflicts, overwrite the credentials files entirely if you need to
# please DM me on slack if you want help resolving conflicts!
git push --force

@@ -13,7 +13,7 @@ class User < ApplicationRecord

before_create :set_default_role

validates :name, presence: true
# validates :name, presence: true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good, actually. This is a good way to keep this branch scoped to what your changes should be.

However, we obviously will wait to deploy this until the blocking, dependent branches have shipped, so you will end up not needing this!

Copy link
Collaborator Author

@trca831 trca831 Dec 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @raquii Before I do this (what you commented above this regarding what to run on my branch), should I do a git pull?

@trca831 trca831 force-pushed the HER-16-Migration-drop-name-column-from-Users branch from 80ba630 to 71b7672 Compare December 5, 2024 02:32
@raquii raquii self-requested a review December 5, 2024 02:32
@trca831 trca831 force-pushed the HER-16-Migration-drop-name-column-from-Users branch from eadaa2a to d2f4022 Compare December 5, 2024 03:22
@trca831 trca831 merged commit 456aafe into main Dec 5, 2024
3 checks passed
@trca831 trca831 deleted the HER-16-Migration-drop-name-column-from-Users branch December 5, 2024 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants