Skip to content
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

[Delegations prereq 7] Make signers addressible by key ID in LocalStore #197

Merged
merged 7 commits into from
Mar 8, 2022
Merged

[Delegations prereq 7] Make signers addressible by key ID in LocalStore #197

merged 7 commits into from
Mar 8, 2022

Conversation

ethan-lowman-dd
Copy link
Contributor

@ethan-lowman-dd ethan-lowman-dd commented Dec 10, 2021 •
edited
Loading

Splitting up #175

Changes:

  1. Move LocalStore struct definition into local_store.go
  2. Refactor the signers ("signers for role") map into two maps, keyIDsForRole and signerForKeyID.
  3. Add SignersForKeyIDs to LocalStore, which will be used to get signers for key IDs specified in a targets delegation

@coveralls
Copy link

coveralls commented Dec 10, 2021 •
edited
Loading

Pull Request Test Coverage Report for Build 1947308688

  • 88 of 94 (93.62%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 70.713%
Changes Missing Coverage Covered Lines Changed/Added Lines %
local_store.go 85 91 93.41%
Totals Coverage Status
Change from base Build 1947284084: 0.5%
Covered Lines: 2231
Relevant Lines: 3155
💛 - Coveralls

@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-7 branch from 84c43e5 to e688697 Compare December 13, 2021 17:57
@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-7 branch from e688697 to 9c34a3c Compare December 14, 2021 23:41
@ethan-lowman-dd
Copy link
Contributor Author

Realized I'm missing new tests here. Will add.

@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-7 branch 2 times, most recently from 5c96ea2 to c8ecd54 Compare January 11, 2022 16:49
@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-6 branch from a39931e to bc74300 Compare January 11, 2022 17:07
@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-7 branch from c8ecd54 to 530e692 Compare January 11, 2022 17:07
@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-6 branch from bc74300 to b8faafb Compare January 11, 2022 17:36
@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-7 branch 2 times, most recently from 81b34fc to a1f2a82 Compare January 18, 2022 16:43
@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-6 branch from 3cef410 to b627120 Compare January 18, 2022 16:48
@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-7 branch 2 times, most recently from 2d5aa82 to e8c3b55 Compare January 19, 2022 15:33
@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-6 branch from 2e447bc to d1fae69 Compare January 19, 2022 15:37
@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-7 branch from e8c3b55 to 7b39db9 Compare January 19, 2022 15:37
asraa
asraa suggested changes Jan 28, 2022
View reviewed changes
Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

@ethan-lowman-dd did you want to add tests?

@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-7 branch 2 times, most recently from bf7753a to 44caf2e Compare January 28, 2022 22:45
@ethan-lowman-dd ethan-lowman-dd marked this pull request as draft January 28, 2022 22:58
@ethan-lowman-dd ethan-lowman-dd marked this pull request as ready for review January 28, 2022 22:59
@ethan-lowman-dd
Copy link
Contributor Author

@ethan-lowman-dd did you want to add tests?

@asraa The functionality of SignersForKeyIDs is tested to some extent in existing tests (since this PR just adds a step of indirection for key lookup) but I will add some dedicated tests too.

asraa
asraa previously approved these changes Feb 9, 2022
View reviewed changes
@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-7 branch from 44caf2e to ff7da54 Compare February 28, 2022 19:40
@ethan-lowman-dd
Copy link
Contributor Author

@asraa Added tests for the changed functionality.

@ethan-lowman-dd ethan-lowman-dd requested review from asraa and mnm678 March 1, 2022 00:06
mnm678
mnm678 previously approved these changes Mar 1, 2022
View reviewed changes
Base automatically changed from ethan.lowman/delegations-prereq-6 to master March 7, 2022 18:37
@ethan-lowman-dd ethan-lowman-dd force-pushed the ethan.lowman/delegations-prereq-7 branch from 160a3ab to eaefa2e Compare March 7, 2022 18:43
@ethan-lowman-dd
Copy link
Contributor Author

Rebasing dismissed reviews. There shouldn't have been any material changes in the rebase.

@ethan-lowman-dd ethan-lowman-dd requested a review from mnm678 March 7, 2022 18:44
@ethan-lowman-dd ethan-lowman-dd merged commit d85e0a2 into master Mar 8, 2022
@ethan-lowman-dd ethan-lowman-dd deleted the ethan.lowman/delegations-prereq-7 branch March 8, 2022 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnm678 mnm678 mnm678 approved these changes

@asraa asraa asraa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ethan-lowman-dd @coveralls @asraa @mnm678