cli/command: internalize and deprecate Cli.NotaryClient#5885
Merged
thaJeztah merged 2 commits intodocker:masterfrom Mar 4, 2025
Merged
cli/command: internalize and deprecate Cli.NotaryClient#5885thaJeztah merged 2 commits intodocker:masterfrom
thaJeztah merged 2 commits intodocker:masterfrom
Conversation
thaJeztah
added
impact/deprecation
status/2-code-review
area/trust
kind/refactor
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5885 +/- ##
=======================================
Coverage 59.27% 59.28%
=======================================
Files 353 354 +1
Lines 29726 29738 +12
=======================================
+ Hits 17620 17630 +10
- Misses 11134 11136 +2
Partials 972 972 |
thaJeztah
changed the title
thaJeztah
added
impact/changelog
area/go-sdk
Benehiko
approved these changes
Mar 4, 2025
The CLI.NotaryClient method is a shallow wrapper around trust.GetNotaryRepository and only depends on the CLI itself to pass its StdErr/StrOut streams. - This patch inlines the code to produce the client, skipping the wrapper. - Define a local interface for some tests where a dummy notary client was used. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This method is a shallow wrapper around trust.GetNotaryRepository, but due to its signature resulted in the trust package, and notary dependencies to become a dependency of the CLI. Consequence of this was that cli-plugins, which need the cli/command package, would also get notary and its dependencies as a dependency. It is no longer used in our code, which constructs the client in packages that need it, so we can deprecate this method. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
internalize_notaryclient
branch
from
70f723b to
9bc16bb
Compare
Member
Author
|
Rebased to get the updated ubuntu-24.04 runners for CI |
Member
Author
|
Let me bring this one in; I have some follow-ups, but didn't want to put them all in a single PR |
This was referenced Mar 4, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cli/command: internalize constructing Notary client
The CLI.NotaryClient method is a shallow wrapper around trust.GetNotaryRepository
and only depends on the CLI itself to pass its StdErr/StrOut streams.
cli/command: deprecate Cli.NotaryClient
This method is a shallow wrapper around trust.GetNotaryRepository, but
due to its signature resulted in the trust package, and notary dependencies
to become a dependency of the CLI. Consequence of this was that cli-plugins,
which need the cli/command package, would also get notary and its
dependencies as a dependency. It is no longer used in our code, which
constructs the client in packages that need it, so we can deprecate this
method.
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)