Skip to content

Migration filename incorrect/missing for Nx0.11.1 #418

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

Closed
ankemp opened this issue Apr 11, 2018 · 6 comments
Closed

Migration filename incorrect/missing for Nx0.11.1 #418

ankemp opened this issue Apr 11, 2018 · 6 comments

Comments

@ankemp
Copy link
Contributor

ankemp commented Apr 11, 2018

The migration: packages/schematics/migrations/20180404-adding-dep-graph.ts created in #406.

The name for this file is: 20180404-adding-dep-graph, yet in #400 20180405-add-workspace-schematic-command is created (which is a more recent date).

And on that note, I can't even find 20180404-adding-dep-graph in the [schematics directory] (https://github.com/nrwl/nx/tree/bc559575f524aacbe3d25eb97210101e60de4526/packages/schematics/migrations).
There are only 22 files in the git repo, but on my local (freshly installed) there are 48 (which you'd have to divide by 2, as there's a JS and TS copy of each).

Found the missing file(s) on git, still having the issue of the migration in Nx0.11.1 being older than the migration in Nx0.10.X

@ankemp ankemp changed the title Migration filename incorrect for Nx0.11.1 Migration filename incorrect/missing for Nx0.11.1 Apr 11, 2018
@vsavkin vsavkin self-assigned this Apr 25, 2018
@vsavkin
Copy link
Member

vsavkin commented Apr 25, 2018

Could you provide more info about the issue? The update command isn't working for you?

@ankemp
Copy link
Contributor Author

ankemp commented Apr 25, 2018

The update command works fine, but it doesn't seem to always run the required migrations.

What I was trying to say was...
In, PR #400 (merged April 5th) you created a migration: 20180405-add-workspace-schematic-command.ts
and in, PR #406 (merged April 6th) you created a migration 20180404-adding-dep-graph.ts

The 406 was merged after 400, but 400 has a newer migration name. So if you were to have updated & ran yarn update before 406 was merged you would have never gotten the migration adding-dep-graph therefor you wouldn't have those scripts in your package.json.

@ankemp
Copy link
Contributor Author

ankemp commented Apr 27, 2018

Might be worth extending the nx-release.js to track what the most recent migration was at the time of the last release, and then rename the prefixes of all the migrations since?

Perhaps using the date is unwise? Maybe ${currentNxVersion}-${incrementedNum}-${migrationName}.ts would be better?
Could even write a script that creates/scaffolds migrations for you.

@vsavkin
Copy link
Member

vsavkin commented May 29, 2018

You are right. We should have used the merge date instead of the creation date.

I'm not sure if it is worth changing nx-migrate because we switched to using the default Angular CLI update mechanism in Nx 6. So we no longer use the nx-migrate command.

I think the default CLI update mechanism has the same issue though, so it might make sense to add a check making sure that a PR doesn't add any migrations to an already released version of Nx (we can use tags for that).

If you submit a PR adding this check to Travis, I'll merge it.

Closing this issue.

@ankemp
Copy link
Contributor Author

ankemp commented May 29, 2018

I'll see what I can do about creating a PR.

@ankemp ankemp closed this as completed May 29, 2018
@github-actions
Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants