Skip to content

feat(core): update to Nx 20#918

Merged
AgentEnder merged 3 commits intonx-dotnet:masterfrom
Lil-Isma:master
Jan 11, 2025
Merged

feat(core): update to Nx 20#918
AgentEnder merged 3 commits intonx-dotnet:masterfrom
Lil-Isma:master

Conversation

@Lil-Isma
Copy link
Copy Markdown
Contributor

@Lil-Isma Lil-Isma commented Jan 5, 2025

This is my attempt to fix the sandbox creation script and update to Nx 20.

In my understanding, Nx 20 has dropped support for NxPluginV1. However, V2 is already supported so I have simply removed all "legacy" code related to V1. This should be fine as V2 is supported since Nx 16.7.0 according to the docs and peer deps are already requiring Nx 18+ today. I'm not sure if this is the right approach and would appreciate some feedback.

Fixes #911

@@ -0,0 +1,23 @@
# path to a directory with all packages
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why this was removed during the migration to Nx 19 but this is required by the sandbox creation script. It seems to be working fine.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I noticed the same thing and did the same recovery as you

for (const ref of change.githubReferences ?? []) {
if (ref.type === 'issue' || ref.type === 'pull-request') {
await handleIssueOrPr(client, ref.value, opts.releaseVersion);
export default class extends DefaultChangelogRenderer {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure how to test this but I think it should work with the new version of the changelog renderer

@EelcoLos
Copy link
Copy Markdown
Contributor

EelcoLos commented Jan 6, 2025

🙏thank you for putting in the effort @Tommy228 .

@sctanner
Copy link
Copy Markdown

sctanner commented Jan 6, 2025

This would be great to have. Thank you!

@AgentEnder AgentEnder marked this pull request as ready for review January 7, 2025 15:03
@nx-cloud
Copy link
Copy Markdown

nx-cloud bot commented Jan 7, 2025

View your CI Pipeline Execution ↗ for commit 6ac3abf.

Command Status Duration Result
nx affected --target lint build test e2e ✅ Succeeded 7m 41s View ↗
nx-cloud record -- yarn documentation:check --v... ✅ Succeeded 8s View ↗
nx run-many -t generate-docs,generate-index --c... ✅ Succeeded 1s View ↗
nx-cloud record -- yarn nx format:check ✅ Succeeded 4s View ↗
nx-cloud record -- yarn commitlint --from 6dc1e... ✅ Succeeded 1s View ↗

☁️ Nx Cloud last updated this comment at 2025-01-10 17:47:07 UTC

Copy link
Copy Markdown
Member

@AgentEnder AgentEnder left a comment

Choose a reason for hiding this comment

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

This looks good 🎉 - one comment. I appreciate jumping in and getting this ball rolling, its something I've been meaning to get around to but parental leave + holidays pushed back quite a bit.

Let's get it green and merged 😄

},
"homepage": "https://nx-dotnet.com/",
"version": "*"
"version": "0.0.0"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change seems unrelated / a bit weird. What's the reason for it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Would this cause an issue? I did this to fix the sandbox creation script where I replaced the old publish-all script with the Nx release programmatic API.

When computing the next version Nx release was not accepting * as the current version so it failed with this error.

 NX   Running release version for project: utils

utils 🔍 Reading data for package "@nx-dotnet/utils" from dist/packages/utils/package.json
utils 📄 Resolved the current version as * from dist/packages/utils/package.json
utils 📄 Using the provided version specifier "99.99.99".

 NX   Invalid semver version "*" provided.

This is ok on the CI/CD as it gets the current version for the latest git tag and ignores the *, but it fails when forking as there is no git tag.

I had to move the git options from nx.json for the same reason otherwise it fails with this error

NX   The "release.git" property in nx.json may not be used with the "nx release version" subcommand or programmatic API. Instead, configure git options for subcommands directly with "release.version.git" and "release.changelog.git".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would this cause an issue? I did this to fix the sandbox creation script where I replaced the old publish-all script with the Nx release programmatic API.

When computing the next version Nx release was not accepting * as the current version so it failed with this error.

 NX   Running release version for project: utils

utils 🔍 Reading data for package "@nx-dotnet/utils" from dist/packages/utils/package.json
utils 📄 Resolved the current version as * from dist/packages/utils/package.json
utils 📄 Using the provided version specifier "99.99.99".

 NX   Invalid semver version "*" provided.

This is ok on the CI/CD as it gets the current version for the latest git tag and ignores the *, but it fails when forking as there is no git tag.

I had to move the git options from nx.json for the same reason otherwise it fails with this error

NX   The "release.git" property in nx.json may not be used with the "nx release version" subcommand or programmatic API. Instead, configure git options for subcommands directly with "release.version.git" and "release.changelog.git".

I recall a similar issue I made a while ago dealing with version '*' as well:
#876 (comment)
Idk if that is related to this?🤷

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@EelcoLos Not related 😄 .

@Tommy228 that sounds fine. Can you take a look at the failing CI tasks?

@Lil-Isma
Copy link
Copy Markdown
Contributor Author

Lil-Isma commented Jan 8, 2025

Failing tests look related to the swagger codegen. I’ll look into it.

@sonarqubecloud
Copy link
Copy Markdown

@Lil-Isma
Copy link
Copy Markdown
Contributor Author

Failing tasks were indeed related to the swagger codegen. The failing test was unrelated to swagger but was missing --skip-swagger-lib so it eventually failed while trying to generate the swagger projects.

I have updated Swashbuckle.AspNetCore to 6.6.2 (which is the default version created by dotnet new and supports .NET 7+) and made some adjustments to the swagger project generator so it should work now.

@AgentEnder AgentEnder merged commit 7573c4e into nx-dotnet:master Jan 11, 2025
@AgentEnder
Copy link
Copy Markdown
Member

@allcontributors-bot add @Tommy228 for code

@AgentEnder
Copy link
Copy Markdown
Member

@all-contributors add @Tommy228 for code

@allcontributors
Copy link
Copy Markdown
Contributor

@AgentEnder

I've put up a pull request to add @Tommy228! 🎉

@mikehaas763
Copy link
Copy Markdown

🎉awesome!

@github-actions
Copy link
Copy Markdown
Contributor

🎉 This PR has been released in 2.5.0 🎉

The release is available on:

Please test and let us know if there are any issues 🎉

@AgentEnder
Copy link
Copy Markdown
Member

@Tommy228 - Thanks again!

@github-actions
Copy link
Copy Markdown
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Migrating to NX v20 breaks dependencies

5 participants