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

feat: implement version command #419

Merged
merged 2 commits into from
Oct 31, 2022
Merged

feat: implement version command #419

merged 2 commits into from
Oct 31, 2022

Conversation

JeyJeyGao
Copy link
Contributor

@JeyJeyGao JeyJeyGao commented Oct 26, 2022 •
edited
Loading

  • Added version command
  • Removed --version flag
  • Update Makefile to set GitCommit value by ldflags
  • Update the BuildMetadata definition and allow user to override the value by environment variable

Please build it bymake build command to include git commit information

Example

$ ./bin/notation version
Notation: Notary v2, A tool to sign, store, and verify artifacts.

Version:     v0.11.0-alpha.4
Go version:  go1.19
Git commit:  f747031135a6010d92c737ea611276b18f4188d7

If it does not build by make build command, the output will be

$ ./bin/notation version
Notation: Notary v2, A tool to sign, store, and verify artifacts.

Version:     v0.11.0-alpha.4
Go version:  go1.19

Signed-off-by: Junjie Gao junjiegao@microsoft.com

@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2022 •
edited
Loading

Codecov Report

Merging #419 (74edd19) into main (f747031) will decrease coverage by 0.51%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #419      +/-   ##
==========================================
- Coverage   34.60%   34.09%   -0.52%     
==========================================
  Files          23       24       +1     
  Lines        1254     1273      +19     
==========================================
  Hits          434      434              
- Misses        809      828      +19     
  Partials       11       11
Impacted Files Coverage Δ
cmd/notation/main.go 0.00% <0.00%> (ø)
cmd/notation/version.go 0.00% <0.00%> (ø)

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

@yizha1 yizha1 added this to the beta-1 milestone Oct 26, 2022
@yizha1 yizha1 added the cli Issue or PR released to Notation CLI label Oct 26, 2022
@JeyJeyGao JeyJeyGao changed the title feat: implement version command feat: implement version command (Draft) Oct 26, 2022
chloeyin
chloeyin reviewed Oct 26, 2022
View reviewed changes
cmd/notation/version.go Outdated Show resolved Hide resolved
chloeyin
chloeyin reviewed Oct 26, 2022
View reviewed changes
cmd/notation/version.go Outdated Show resolved Hide resolved
@JeyJeyGao JeyJeyGao force-pushed the junjiegao/cmd_version branch 3 times, most recently from 133861c to 74edd19 Compare October 26, 2022 08:49
@JeyJeyGao JeyJeyGao changed the title feat: implement version command (Draft) feat: implement version command Oct 26, 2022
@JeyJeyGao
feat: implement version command
ba52b41 
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
@yizha1 yizha1 requested a review from a team October 27, 2022 05:18
@yizha1 yizha1 linked an issue Oct 27, 2022 that may be closed by this pull request
Add version command for notation CLI #345
Closed
@yizha1 yizha1 removed this from the beta-1 milestone Oct 27, 2022
shizhMSFT
shizhMSFT requested changes Oct 27, 2022
View reviewed changes
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

According to the spec, we should always print out all three of Version, Go Version, and Git Commit.

If the commit is not tagged, we should use unreleased as the build metadata.

@JeyJeyGao
fix: resolve comment
c73aeb7 
if BUILD_METADATA is not defined in ENVIRONMENT variable, use
`unreleased` as the value

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
@JeyJeyGao
Copy link
Contributor Author

According to the spec, we should always print out all three of Version, Go Version, and Git Commit.

If the commit is not tagged, we should use unreleased as the build metadata.

  1. if GitCommit is not set by compiler, its output line will be skipped, which is consistent with oras version behavior and confirmed by Yi @yizha1
  2. updated: build metadata will be unreleased if user didn't define the BUILD_METADATA environment variable and didn't set the tag.

@shizhMSFT

patrickzheng200
patrickzheng200 approved these changes Oct 28, 2022
View reviewed changes
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

shizhMSFT
shizhMSFT approved these changes Oct 31, 2022
View reviewed changes
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@shizhMSFT shizhMSFT merged commit a219ad5 into notaryproject:main Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chloeyin chloeyin chloeyin left review comments

@patrickzheng200 patrickzheng200 patrickzheng200 approved these changes

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@rgnote rgnote rgnote approved these changes

@priteshbandi priteshbandi Awaiting requested review from priteshbandi priteshbandi is a code owner

Assignees

@JeyJeyGao JeyJeyGao

Labels
cli Issue or PR released to Notation CLI
Projects
Notary Project Planning Board
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add version command for notation CLI
7 participants
@JeyJeyGao @codecov-commenter @rgnote @chloeyin @shizhMSFT @patrickzheng200 @yizha1