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

Remove old admin column from users table #2000

Merged
merged 3 commits into from Jan 5, 2023
Merged

Conversation

eddierubeiz
Copy link
Contributor

@eddierubeiz eddierubeiz commented Dec 16, 2022

Ref #1948
Remove the now-obsolete admin column and the corresponding admin? user method.

This is our first migration since we moved to Rails 7, so this PR also includes some unrelated changes to schema.rb. Notably, the default precision for datetime columns is changing from 0 to 6. (more info)

See also: rails/rails#44286

@eddierubeiz eddierubeiz marked this pull request as ready for review January 5, 2023 16:59
@jrochkind
Copy link
Contributor

jrochkind commented Jan 5, 2023

Thanks for figuring out the precision/Rails 7 stuff, and for link. I haven't completely wrapped my head around all details.

You didn't make manual changes to the schema.rb, that changes are the result of letting Rails migrate it, correct? So you're confident it accurately represents our current database?

Approved assuming that's so.

@jrochkind
Copy link
Contributor

Ideally, the commit message for the schema change would include more information about how those changes were made, as well as a link to the relevant Rails docs there.

That's more a note for the future than a requirement you rewrite the commit message now. 🤷

remove obsolete admin? method
note: this is our first rails migration since we moved to rails 7, so schema.rb specifies the rails version and changes the precision default for datetime columns.
see:
https://rubyonrails.org/2022/2/11/this-week-in-rails-rails-7-0-2-schema-versioning-based-on-the-rails-version-and-more-cbcb0592
rails/rails#44286
@eddierubeiz
Copy link
Contributor Author

Confirmed -- the changes were automatic and would have been applied to the schema file in any case.
I did edit the commit message - easy enough in this particular case.

@eddierubeiz eddierubeiz merged commit b1b14d6 into master Jan 5, 2023
@eddierubeiz eddierubeiz deleted the remove_old_admin_column branch January 5, 2023 17:40
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.

None yet

2 participants