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

Baking snapshot with autoid #962

Open
dereuromark opened this issue Nov 4, 2023 · 5 comments
Open

Baking snapshot with autoid #962

dereuromark opened this issue Nov 4, 2023 · 5 comments

Comments

@dereuromark
Copy link
Member

Description

When baking

bin/cake bake migration_snapshot Init

The result contains already

public bool $autoId = false;

But there is a specific flag for it, so I would assume it wouldnt do that for us out of the box.
Otherwise we need the opposite flag enable-autoid:

--disable-autoid      Disable phinx behavior of automatically adding an
                      id field.

Bake Version

3.0.4

PHP Version

PHP 8.2

@dereuromark dereuromark added this to the 3.x (CakePHP 5) milestone Nov 4, 2023
@ndm2
Copy link
Contributor

ndm2 commented Nov 4, 2023

That is the intended behavior when there are primary keys that conflict with the Migrations.unsigned_primary_keys config, it's the continuation of the old behavior of unsigned primary keys causing the option to be generated.

Maybe it's possible to drop this behavior, and treat incompatible primary keys as "special primary keys" just like for example composite primary keys, for which table() call options are generated accordingly, but I'm not sure, with migrations there's always 5 implications lurking around the corner, just waiting to smash your kneecaps.

@dereuromark
Copy link
Member Author

dereuromark commented Nov 4, 2023

Could we switch the config to --enable-autoid and then force the unsigned keys where needed or alike?
Maybe to be safe another option --force-unsigned on top here if needed?

@ndm2
Copy link
Contributor

ndm2 commented Nov 6, 2023

I'm not sure, maybe. Figuring out the technical implications and possible unwanted side effects will take some time.

@dereuromark
Copy link
Member Author

Well, the migrations before were with autoid, and I would expect the merged one to be the same.
If we need to force this somehow, that sounds reasable to me.

Copy link

github-actions bot commented Mar 8, 2024

This issue is stale because it has been open for 120 days with no activity. Remove the stale label or comment or this will be closed in 15 days

@github-actions github-actions bot added the stale label Mar 8, 2024
@dereuromark dereuromark added pinned and removed stale labels Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants