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.

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

doc: add missing username/password options to commands #293

Merged
merged 3 commits into from
Aug 17, 2022

Conversation

binbin-li
Copy link
Contributor

@binbin-li binbin-li commented Aug 10, 2022

What?

Resovles #276

For commands other than notation login, we should allow them support username/password options even without first-time logging.
Since the login command requires credentialsStore set up which might be missing for users. So we cannot force users always use login to save credentials. Other commands should be able to use the credentials directly from input without credentialsStore set up.

Updated commands:

  1. sign
  2. push
  3. pull
  4. list
  5. cache
  6. verify

Signed-off-by: Binbin Li [email protected]

@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2022

Codecov Report

Merging #293 (eebfa63) into main (b5b56e0) will decrease coverage by 30.42%.
The diff coverage is n/a.

❗ Current head eebfa63 differs from pull request most recent head 262cc16. Consider uploading reports for the commit 262cc16 to get more accurate results

@@             Coverage Diff             @@
##             main     #293       +/-   ##
===========================================
- Coverage   61.07%   30.64%   -30.43%     
===========================================
  Files           9       25       +16     
  Lines         280     1612     +1332     
===========================================
+ Hits          171      494      +323     
- Misses        102     1105     +1003     
- Partials        7       13        +6     
Impacted Files Coverage Δ
cmd/notation/list.go 42.10% <0.00%> (ø)
cmd/notation/login.go 43.20% <0.00%> (ø)
cmd/notation/pull.go 18.68% <0.00%> (ø)
cmd/notation/logout.go 54.16% <0.00%> (ø)
cmd/notation/push.go 24.61% <0.00%> (ø)
cmd/notation/main.go 0.00% <0.00%> (ø)
cmd/notation/registry.go 0.00% <0.00%> (ø)
cmd/notation/key.go 23.52% <0.00%> (ø)
cmd/notation/plugin.go 0.00% <0.00%> (ø)
cmd/notation/cache.go 23.38% <0.00%> (ø)
... and 6 more

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

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

Copy link
Contributor

@dtzar dtzar left a comment

Choose a reason for hiding this comment

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

It would be nice to add to the login section something like: "Once login is completed, then -u -p is no longer required for any notation commands against the registry server authenticated."

@dtzar
Copy link
Contributor

dtzar commented Aug 16, 2022

@yizha1 - since this is just a doc fix and is now at the 1 week mark, I recommend merging it today before notation release.

@binbin-li
Copy link
Contributor Author

It would be nice to add to the login section something like: "Once login is completed, then -u -p is no longer required for any notation commands against the registry server authenticated."

Good call, I'll add it.

Signed-off-by: Binbin Li <[email protected]>
@binbin-li binbin-li force-pushed the fix-cli-spec branch 2 times, most recently from eebfa63 to 0929fd3 Compare August 17, 2022 02:23
Signed-off-by: Binbin Li <[email protected]>
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 6ede6db into notaryproject:main Aug 17, 2022
chloeyin added a commit that referenced this pull request Aug 25, 2022
* doc: add missing username/password options to commands (#293)

Signed-off-by: Binbin Li <[email protected]>

* bump up version to v0.10.0-alpha.3 (#301)

* bump up version to v1.0.0-alpha.3
* revise version to v0.10.0-alpha.3

Signed-off-by: Yi Zha <[email protected]>

* fix: update notation-go (#294)

Signed-off-by: Junjie Gao <[email protected]>

* Build: Bump dependencies (#306)

Signed-off-by: Yi Zha <[email protected]>

* feat: add weekly release (#282)

Signed-off-by: Junjie Gao <[email protected]>

Signed-off-by: Binbin Li <[email protected]>
Signed-off-by: Yi Zha <[email protected]>
Signed-off-by: Junjie Gao <[email protected]>
Co-authored-by: Binbin Li <[email protected]>
Co-authored-by: Yi Zha <[email protected]>
Co-authored-by: Junjie Gao <[email protected]>
Co-authored-by: zaihaoyin <[email protected]>
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.

4 participants