-
-
Notifications
You must be signed in to change notification settings - Fork 9
feat(req-param): migrate req-param to codemod.com #94
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
base: main
Are you sure you want to change the base?
Conversation
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.
Basically, this code is repeated from the previous codemods, probably could be factored out into a utils package, though I'm wondering how this would behave for codemod registration, being a separate package and not published to npm
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.
a local npm workspace will be bundled by the codemod cli.
!the ci have to npm I before publish!
| @@ -0,0 +1,22 @@ | |||
| { | |||
| "name": "@expressjs/req-param", | |||
| "private": true, | |||
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.
By the way, I'm setting this to private so we don't accidentally publish this to npm, but does this affect codemod registration?
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.
codemod cli don't care about package.json
AugustinMauroy
left a comment
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.
okay on userland migration we require some test around import idk what do you want but here the list
- cjs
- cjs destructred
- esm dynamic import with
const foo = await importand in C.B. - esm alias
| @@ -0,0 +1,23 @@ | |||
| schema_version: "1.0" | |||
| name: "@expressjs/req-param" | |||
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.
I'm not sure of the naming because it's too general. So it's may confuse user
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.
Naming codemods is just as hard as naming variables, a difficult decision.
I’ll think about other names.
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.
BTW I use gh copilot for that question and not same too bad. if you want I have gh pro for free as student I can ask.
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.
Yeah, i also have github pro for free as a student. i’ll ask them for some good name suggestions.
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.
explicit-request-params? req-param-migration?
I don’t quite understand what you mean by the imports. We don’t need to modify imports here in the codemods. |
Oh right, my bad just simple ESM should work 😅 |
AugustinMauroy
left a comment
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.
LGMT !
New codemod migrated to use ast-grep and will be published soon on the codemod.com platform