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

Dev environment should start creating users with id 2 #11807

Closed
wants to merge 1 commit into from

Conversation

cam-peck
Copy link

Will start adding users at id=2 ( leaving id=1 open for guest ) going forward in the phpbb_users table.

https://dev.mysql.com/doc/refman/8.4/en/example-auto-increment.html

@cam-peck cam-peck changed the title Dev environment should start creating users with id 2 #11785 Dev environment should start creating users with id 2 Jan 22, 2025
Copy link
Collaborator

@nanaya nanaya left a comment

Choose a reason for hiding this comment

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

updating the initial table creation's migration file (by adding ->from(2)) is probably fine...

@cam-peck
Copy link
Author

Hrmm... The idea being migrations are not re-run in prod, so only developers will see the changes moving forward when they do a fresh re-seed & migrate?

This would also work, but will result in the migration inside that file and the prod DB in different states. I do like the simplicity of the from(2) change, but I feel a new migration better represents the change history. It also avoids any backwards-compatibility issues with modifying an existing migration -- if we wanted to add these changes to prod in the future, we'd have to run the down for the original migration, which removes a bunch of tables.

@nanaya What are your thoughts? Am I being too picky here?

@nanaya
Copy link
Collaborator

nanaya commented Jan 22, 2025

the value itself changes the moment any data is entered. Rolling back doesn't actually do anything either (it'll just set to the minimum valid value).

@cam-peck
Copy link
Author

I'm confused.

What is your reasoning on ->from(2) vs a new migration?

I feel a new migration is better because:

  1. A new migration better represents the change history. In 2015, we didn't need to start from id=2. Now, in 2025, we do. The migration fits that storyline clearer than a git blame check in an old migration file.
  2. For any schema transformations, it's best practice to run a new migration for the schema changes. I understand that we don't want to run this on prod ( which is the main reason for this practice ), but it feels 'icky to have a different state in a migration file from the prod DB.

@nanaya
Copy link
Collaborator

nanaya commented Jan 22, 2025

  1. A new migration better represents the change history. In 2015, we didn't need to start from id=2. Now, in 2025, we do. The migration fits that storyline clearer than a git blame check in an old migration file.

more like it has always kind of bugged but ignored for a long time.

The migration files also have been modified multiple times and the initial migration didn't match the actual db either.

there's currently no migration flattening and more migration files means longer setup (and test) time

  1. For any schema transformations, it's best practice to run a new migration for the schema changes. I understand that we don't want to run this on prod ( which is the main reason for this practice ), but it feels 'icky to have a different state in a migration file from the prod DB.

auto_increment isn't a static value. It changes any time data is inserted. It's different on all non-empty db. Running the alter on prod db won't do anything (both up and down), nor on any existing db with non-0 users.

@cam-peck
Copy link
Author

I see I see, this makes sense with more project context. Both of my points resolve around DB vs. migration state.

I’ll make the update to ‘from(2)’ when I’m off this afternoon — thanks for the rundown!

Copy link
Collaborator

@notbakaneko notbakaneko left a comment

Choose a reason for hiding this comment

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

I'm on board with just updating the initial migration

@nanaya
Copy link
Collaborator

nanaya commented Jan 29, 2025

Done in #11828

@nanaya nanaya closed this Jan 29, 2025
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.

3 participants