-
Notifications
You must be signed in to change notification settings - Fork 7
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
refactor: use the new refactored migrateSituation from @publicodes/tools
#577
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Report for the pull request #577🌐 Translation statusUI's texts
FAQ's questions
|
Pour le moment, nous n'avons pas de tests unitaires sur NGC directement, en général, je teste avec différentes simulations, par exemple:
|
"allowJs": true, | ||
"skipLibCheck": true, | ||
"strict": true, | ||
"noEmit": true, | ||
"esModuleInterop": true, | ||
"module": "esnext", | ||
"moduleResolution": "node", | ||
"moduleResolution": "bundler", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pourquoi cette modif ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'avais des erreurs du LS en utilisant @publicodes/tools
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je me demande si le problème ne devrait pas être réglé dans publicodes/tools plutôt qu'ici, tu crois pas ? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bundler est la valeur par défaut d'une install create-next-app donc ça me va (j'ai une compréhension assez faible de ce que ça impacte)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
convertSimulation est nécessaire (il gère la migration de la reste de l'objet simulation). Par contre je vois qu'il n'était déjà plus appelé dans migrateSimulation
Je ne trouve aucune occurrences de |
0bdc1eb
to
7cf5757
Compare
This PR is the follow-up to the migration module refactoring from
@publicodes/tools
done in publicodes/tools#43 (the release must be done before being able to merge).Important
I quickly tested by artificially adding keys to the model's migration instructions and looking the situation in the local storage. Is there a more precise test protocol?
Changelog
migrateSituation
API.Migration
type provided by@publicodes/tools
instead of theMigrationType
.Situation
type provided bypublicodes
.publicodes-state/helpers/migration
code.