Skip to content

Conversation

@simo5
Copy link
Member

@simo5 simo5 commented Mar 13, 2025

The migration tool automatically changed some of thcode causing some churn. I manually changed some of the things, like removing expr_2021 from macros as it was not necessary and changing the renaming of the now reserved word gen from r#gen to generator.

A bunch of let Some() {} else () expressions were changed to matches, I let that be, sometime smatch turns out to be idiomatic, sometimes it is about the same readability.

Checklist

  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation was updated
  • This is not a code change

Reviewer's checklist:

  • Any issues marked for closing are fully addressed
  • There is a test suite reasonably covering new functionality or modifications
  • This feature/change has adequate documentation added
  • A changelog entry is added if the change is significant
  • Code conform to coding style that today cannot yet be enforced via the check style test
  • Commits have short titles and sensible text

The migration tool automatically changed some of thcode causing some
churn. I manually changed some of the things, like removing expr_2021
from macros as it was not necessary and changing the renaming of the now
reserved word gen from r#gen to generator.

A bunch of let Some() {} else () expressions were changed to matches, I
let that be, sometime smatch turns out to be idiomatic, sometimes it is
about the same readability.

Signed-off-by: Simo Sorce <[email protected]>
@simo5 simo5 requested a review from Jakuje March 13, 2025 20:37
Copy link
Contributor

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

lgmt. I find the matches most of the time easier to read than the if let Ok() ... constructs, but at some when there is a lot of code inside, they end up too indented ...

@simo5 simo5 merged commit 606d9ec into latchset:main Mar 13, 2025
24 checks passed
@Jakuje
Copy link
Contributor

Jakuje commented Mar 17, 2025

Should the edition be changed also in Cargo.toml ?

https://github.com/latchset/kryoptic/blob/main/Cargo.toml#L4

@simo5
Copy link
Member Author

simo5 commented Mar 18, 2025

Should the edition be changed also in Cargo.toml ?

https://github.com/latchset/kryoptic/blob/main/Cargo.toml#L4

Well, I am confused, the migration is really about changing it in Cargo.toml which is how I started it ... I am really surprised the MR did not end up having it there, without the Cargo.toml change no change really happened. I will open a followup change to actually do the change then.

@simo5
Copy link
Member Author

simo5 commented Mar 18, 2025

Filed #187

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants