raito/lanzaboote
0
0
Fork 0
Code Issues 45 Pull requests 6 Projects Releases 6 Packages Wiki Activity Actions

shared: generalize signature schemes #303

Merged
RaitoBezarius merged 9 commits from refactor-signature into master 2024-09-03 16:04:02 +00:00
Conversation 11 Commits 9 Files changed 9 +334 -156
RaitoBezarius commented 2024-02-11 15:45:14 +00:00 (Migrated from github.com)
Copy link

Split off #278. Contains only signature generalization contents and misc refactoring to make things easier for the remote signing PR.

Split off #278. Contains only signature generalization contents and misc refactoring to make things easier for the remote signing PR.
RaitoBezarius commented 2024-02-12 23:49:13 +00:00 (Migrated from github.com)
Copy link

@blitz I added more documentation (overview too), let me know if this is what you had in mind or where would you like me to expand.

@blitz I added more documentation (overview too), let me know if this is what you had in mind or where would you like me to expand.
blitz (Migrated from github.com) reviewed 2024-03-19 08:48:48 +00:00
rust/tool/shared/src/signature/README.md
blitz (Migrated from github.com) commented 2024-03-19 08:48:48 +00:00
Copy link

This documentation is nice, but it looks like it should be the module documentation for the signature module and not a standalone README. Part of this can also go away, because it duplicates documentation of the LanzabooteSigner trait.

This documentation is nice, but it looks like it should be the module documentation for the signature module and not a standalone README. Part of this can also go away, because it duplicates documentation of the `LanzabooteSigner` trait.
blitz (Migrated from github.com) reviewed 2024-03-19 08:49:22 +00:00
rust/tool/shared/src/signature/mod.rs
blitz (Migrated from github.com) commented 2024-03-19 08:49:22 +00:00
Copy link

nit: This could just be Signer. Otherwise we can start to prefix everything with the project name. :)

nit: This could just be `Signer`. Otherwise we can start to prefix everything with the project name. :)
blitz (Migrated from github.com) reviewed 2024-03-19 08:54:15 +00:00
rust/tool/shared/src/signature/local.rs
blitz (Migrated from github.com) commented 2024-03-19 08:54:15 +00:00
Copy link

If !success means "either not signed or something else went wrong", we might not want to treat this as an error. But I also understand the other side of the argument.

If `!success` means "either not signed or something else went wrong", we might not want to treat this as an error. But I also understand the other side of the argument.
blitz (Migrated from github.com) requested changes 2024-03-19 08:57:00 +00:00
blitz (Migrated from github.com) left a comment
Copy link

Looks good! I liked the clear commit history, which made this easier to review.

Will approve when the merge conflicts are resolved. Feel free to address any of the other nitpicky comments as well or ignore them.

Looks good! I liked the clear commit history, which made this easier to review. Will approve when the merge conflicts are resolved. Feel free to address any of the other nitpicky comments as well or ignore them.
RaitoBezarius (Migrated from github.com) reviewed 2024-08-28 09:25:11 +00:00
rust/tool/shared/src/signature/local.rs
RaitoBezarius (Migrated from github.com) commented 2024-08-28 09:25:11 +00:00
Copy link

OK, then, I won't bubble it.

OK, then, I won't bubble it.
RaitoBezarius commented 2024-08-28 09:27:23 +00:00 (Migrated from github.com)
Copy link

@blitz This is ready for a review again, I think.

@blitz This is ready for a review again, I think.
blitz (Migrated from github.com) approved these changes 2024-08-31 12:48:35 +00:00
blitz (Migrated from github.com) left a comment
Copy link

I don't have time for a deep review. I like the documentation. Looks like a useful refactoring. @nikstur Are you ok with this?

I don't have time for a deep review. I like the documentation. Looks like a useful refactoring. @nikstur Are you ok with this?
nikstur (Migrated from github.com) reviewed 2024-09-03 15:24:28 +00:00
nikstur (Migrated from github.com) left a comment
Copy link

Good to go from my end after you removed tests/lib.nix

Good to go from my end after you removed tests/lib.nix
RaitoBezarius (Migrated from github.com) reviewed 2024-09-03 15:25:49 +00:00
nikstur (Migrated from github.com) approved these changes 2024-09-03 16:03:03 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
blitz
nikstur
Labels
Clear labels   
bug

Something isn't working

  
dependency

Issues related to our dependencies

  
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
enhancement

New feature or request

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
question

Further information is requested

  
review-next

   
security

   
stub

   
tool

   
wontfix

This will not be worked on

No labels
bug
dependency
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
review-next
security
stub
tool
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: raito/lanzaboote#303
No description provided.
Delete branch "refactor-signature"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?