Skip to content

feat(crypto-fips): add migration recipe for crypto.fips #177

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
Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open

feat(crypto-fips): add migration recipe for crypto.fips #177

wants to merge 26 commits into from

Conversation

max-programming
Copy link
Contributor

Closes #175

@max-programming
feat(crypto-fips): add migration recipe for transitioning from crypto…
39f92b0 
….fips to crypto.getFips() and crypto.setFips()
@max-programming
Copy link
Contributor Author

Let me know if the settings.json change feels unnecessary, if that's the case, I'll remove it

Personally, I think having that helps

@avivkeller
Copy link
Member

Let me know if the settings.json change feels unnecessary, if that's the case, I'll remove it

Personally, I think having that helps

You don't have to remove it entirely, but can it be a separate PR (to keep our commits organized)?

Thank you!

avivkeller
avivkeller approved these changes Aug 13, 2025
View reviewed changes
@max-programming
Copy link
Contributor Author

@avivkeller Alright then. I'm gonna make another PR for another recipe where I'll include this change as a separate commit

@max-programming
chore(vscode): revert settings json file to default
17c3bc9
@max-programming
refactor(crypto-fips): streamline crypto base path retrieval and impr…
078fecd 
…ove code readability
@max-programming
refactor(crypto-fips): use generators for crypto base path collection
7cd8777 
Replaces array concatenation with generator-based iteration
to improve readability, memory usage, and performance.
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy approved these changes Aug 13, 2025
View reviewed changes
Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

WOW 😵‍💫 that's an awesome first contribution. I love your usage of yield.

Thanks you for your contribution, I hope that you had enjoyed it. If you have any feedback feel it

@max-programming
Copy link
Contributor Author

Thanks @AugustinMauroy!

I'll update the suggested changes
Also there's one two more test cases that are failing which I'll add and re-request a review

@max-programming
refactor(crypto-fips): implement transformation for crypto.fips to ge…
b4d9c0c 
…tFips/setFips

This update introduces a transformation for replacing deprecated `crypto.fips` calls with `crypto.getFips()` and `crypto.setFips()`. It includes enhancements to variable collection, import specifier updates, and destructuring adjustments to ensure proper usage of the new methods. Additionally, new test cases have been added to validate the changes.
@max-programming
Copy link
Contributor Author

@AugustinMauroy I think some functions I used can be put into utils. Ones that are used to get import specifiers and all
Let me know which ones can be re-used in other codemods

AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
@max-programming
Copy link
Contributor Author

For the utility functions thing, I'll examine those and probably open another PR for that specifically

@AugustinMauroy
Copy link
Member

No problem !

@JakobJingleheimer JakobJingleheimer added the awaiting author Reviewer has requested something from the author label Sep 23, 2025
@AugustinMauroy
Copy link
Member

@max-programming any news ?

@max-programming
Copy link
Contributor Author

@AugustinMauroy I apologise for the huge delay. Today's my last day for college assignment submissions. I'm gonna have this as my first priority once that's over

Hopefully won't take long

@AugustinMauroy
Copy link
Member

@AugustinMauroy I apologise for the huge delay. Today's my last day for college assignment submissions. I'm gonna have this as my first priority once that's over

Hopefully won't take long

I completely understand. I am also at college studying application development.

@max-programming
Copy link
Contributor Author

I hope everything goes well this time around haha

@JakobJingleheimer JakobJingleheimer added the dep:v23 Migration handles deprecation introduced in node v23 label Oct 16, 2025
@AugustinMauroy AugustinMauroy added awaiting reviewer Author has responded and needs action from the reviewer and removed awaiting author Reviewer has requested something from the author labels Oct 18, 2025
brunocroh
brunocroh requested changes Oct 19, 2025
View reviewed changes
Copy link
Member

@brunocroh brunocroh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

We’ll use updateBinding, which already exists in our utility library, instead of recreating it here.

So, we need to add support for updateBinding to add multiple bindings (and maybe remove as well) at the same time.

@max-programming
Copy link
Contributor Author

max-programming commented Oct 19, 2025 •
edited
Loading

Not sure why these checks fail

How can we get them to pass?

@AugustinMauroy
Copy link
Member

Not sure why these checks fail

How can we get them to pass?

because you touch to the utility and you break something

@max-programming
Copy link
Contributor Author

Ah I think I found the failing part

@max-programming
Copy link
Contributor Author

Passed finally

This was such a headache lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AugustinMauroy AugustinMauroy AugustinMauroy approved these changes

@avivkeller avivkeller Awaiting requested review from avivkeller

@brunocroh brunocroh Awaiting requested review from brunocroh

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

awaiting reviewer Author has responded and needs action from the reviewer dep:v23 Migration handles deprecation introduced in node v23

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: DEP0093: crypto.fips is deprecated and replaced

5 participants

@max-programming @avivkeller @AugustinMauroy @brunocroh @JakobJingleheimer