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.

Sign up for GitHub

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

Jump to bottom

chore: 🤖 upgrade to nx17 #734

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

chore: 🤖 upgrade to nx17 #734

wants to merge 2 commits into from

Conversation

alexdabast
Copy link

Upgrading to nx17 using npx nx migrate latest

✅ Closes: #731

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • [ x] Other... Please describe:
    Nx upgrade version

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

I did not manually bump any devPeer in libs folder as I don't think it is required to do that

@alexdabast
chore: 🤖 upgrade to nx17
d1a564d 
Upgrading to nx17 using npx nx migrate latest

✅ Closes: jsverse#731
@stackblitz StackBlitz
Copy link

stackblitz bot commented Dec 8, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@shaharkazaz
Copy link
Collaborator

shaharkazaz commented Dec 8, 2023 •
edited

@alexdabast Thanks for the PR!
Note that the lint & tests are failing.
Regarding the lint, I think some warnings are now errors, can you adjust the lint to act as before the update?
Also, can you run the Angular 17 migration on the playground?

@alexdabast
Copy link
Author

@shaharkazaz I will look into the tests failing
however what do you mean by migration the playground ?
It looks to me that the playground is using angular 17 as you can use the new syntaxe for example and it is working fine
image

and the code
image

@shaharkazaz
Copy link
Collaborator

I'll clarify, I meant running the new syntax migration so that the playground code will use it ☺️

@alexdabast
chore: 🤖 Migration html template a17 and fix CI:TESTs
82ce475 
Html template of the playground has been migrated using angular cli ng
generate @angular/core:control-flow
@alexdabast
Copy link
Author

@shaharkazaz I have run the migration and did some test seems ok
Regarding the lint and test now angular workspace are by default standalone so I force the workspace creation to standalone:false and removed the standalone:true on the standalone test.

Also had to replace runSchematicAsync by runSchematic and remove the conversion to promise as it returns now directly a promise

@shaharkazaz
Copy link
Collaborator

shaharkazaz commented Dec 20, 2023 •
edited

@alexdabast

  1. Lint: Please set @typescript-eslint/no-explicit-any as a warning and not an error, and remove all the unused var cases, that should take care of that.
  2. Tests: Seems like a beforeAll hook is failing, try to run it locally and debug the issue, any chance you need to update from master?

@theguidingstar
Copy link

Any news on this?
When it's going to be merged and released?

@jontze
Copy link

jontze commented Mar 11, 2024

Thanks for your work on this PR, @alexdabast. Do you still have the time to push this forward?

If not, I would be happy to support you in completing the remaining points :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Chore: 🤖 upgrade to angular v17
4 participants
@alexdabast @shaharkazaz @theguidingstar @jontze