Skip to content

Comments

feat: support tagged reference#359

Merged
binbin-li merged 2 commits intonotaryproject:dev-rc.1from
binbin-li:refactor-verify
Sep 28, 2022
Merged

feat: support tagged reference#359
binbin-li merged 2 commits intonotaryproject:dev-rc.1from
binbin-li:refactor-verify

Conversation

@binbin-li
Copy link
Contributor

@binbin-li binbin-li commented Sep 28, 2022

What?

  1. Support passing in a reference in the format: registry/repository:v1. Before that, only a reference with digest tag is supported.
  2. Refactor the print out format to be more readable.

Test

Success case:
image

Failure case:
failure

Signed-off-by: Binbin Li libinbin@microsoft.com

@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2022

Codecov Report

Merging #359 (0d2f8d7) into dev-rc.1 (4bc23db) will decrease coverage by 0.10%.
The diff coverage is 11.11%.

@@             Coverage Diff              @@
##           dev-rc.1     #359      +/-   ##
============================================
- Coverage     31.26%   31.15%   -0.11%     
============================================
  Files            26       26              
  Lines          1705     1711       +6     
============================================
  Hits            533      533              
- Misses         1155     1161       +6     
  Partials         17       17              
Impacted Files Coverage Δ
cmd/notation/verify.go 30.50% <11.11%> (-3.46%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@binbin-li binbin-li force-pushed the refactor-verify branch 2 times, most recently from 9163e45 to c1635f5 Compare September 28, 2022 02:36
@JeyJeyGao
Copy link
Contributor

LGTM

@yizha1
Copy link
Contributor

yizha1 commented Sep 28, 2022

Regarding the success case, is there an empty line before the digest? If yes, I suggest removing the empty line.

@binbin-li
Copy link
Contributor Author

Regarding the success case, is there an empty line before the digest? If yes, I suggest removing the empty line.

that's the old screenshot, I've updated it.

@chloeyin
Copy link
Contributor

BTW, could you use cmd.ParseFlagPluginConfig to parse the plugin config?

pluginConfig, err := cmd.ParseFlagPluginConfig(opts.pluginConfig)

We don't need to parse the config string manully.

Signed-off-by: Binbin Li <libinbin@microsoft.com>
@binbin-li
Copy link
Contributor Author

BTW, could you use cmd.ParseFlagPluginConfig to parse the plugin config?

pluginConfig, err := cmd.ParseFlagPluginConfig(opts.pluginConfig)

We don't need to parse the config string manully.

Sure, thanks for calling it out.

Signed-off-by: Binbin Li <libinbin@microsoft.com>
Copy link
Contributor

@patrickzheng200 patrickzheng200 left a comment

Choose a reason for hiding this comment

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

LGTM

@binbin-li binbin-li merged commit 6ad8f99 into notaryproject:dev-rc.1 Sep 28, 2022
@binbin-li binbin-li deleted the refactor-verify branch September 28, 2022 06:10
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.

6 participants