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: make --identity-token an alias of --password #1294

Merged
merged 9 commits into from
Apr 12, 2024
Merged

feat: make --identity-token an alias of --password #1294

merged 9 commits into from
Apr 12, 2024

Conversation

wangxiaoxuan273
Copy link
Contributor

@wangxiaoxuan273 wangxiaoxuan273 commented Mar 18, 2024 •
edited by qweeah
Loading

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #742

Please check the following list:

  • Does the affected code have corresponding tests, e.g. unit test, E2E test?
  • Does this change require a documentation update?
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have an appropriate license header?

@wangxiaoxuan273
Copy link
Contributor Author

wangxiaoxuan273 commented Mar 18, 2024 •
edited
Loading

Current display:
image
issue
image

@codecov Codecov
Copy link

codecov bot commented Mar 18, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 84.31373% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 84.49%. Comparing base (bb3443d) to head (46bfbe3).

Files Patch % Lines
cmd/oras/internal/option/remote.go 89.74% 3 Missing and 1 partial ⚠️
cmd/oras/root/login.go 42.85% 0 Missing and 4 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1294      +/-   ##
==========================================
+ Coverage   84.40%   84.49%   +0.08%     
==========================================
  Files         101      101              
  Lines        3617     3637      +20     
==========================================
+ Hits         3053     3073      +20     
  Misses        335      335              
  Partials      229      229

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@qweeah
Copy link
Contributor

qweeah commented Mar 19, 2024 •
edited
Loading

-u xxx --identity-token xxx usage should be banned, see #742 (comment) <= just find this has already been resolved, good catch.

qweeah
qweeah reviewed Mar 19, 2024
View reviewed changes
cmd/oras/root/login.go Outdated Show resolved Hide resolved
qweeah
qweeah reviewed Mar 19, 2024
View reviewed changes
cmd/oras/root/login.go Outdated Show resolved Hide resolved
@TerryHowe
Copy link
Member

Closes: #1040

qweeah
qweeah approved these changes Mar 21, 2024
View reviewed changes
Copy link
Contributor

@qweeah qweeah left a comment

Choose a reason for hiding this comment

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

LGTM, created #1301 as a follow up so we can make the flag confliction check easier.

@shizhMSFT
Copy link
Contributor

LGTM, created #1301 as a follow up so we can make the flag confliction check easier.

Can we resolve #1301 first and then this PR since it really simplifies the code a lot?

@qweeah qweeah mentioned this pull request Mar 28, 2024
Closed
qweeah
qweeah requested changes Apr 9, 2024
View reviewed changes
cmd/oras/internal/option/remote.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/remote.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/remote.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/remote.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/remote.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/remote.go Outdated Show resolved Hide resolved
qweeah
qweeah approved these changes Apr 10, 2024
View reviewed changes
Copy link
Contributor

@qweeah qweeah 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 reviewed Apr 11, 2024
View reviewed changes
cmd/oras/internal/option/remote.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/remote.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/remote.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/remote.go Outdated Show resolved Hide resolved
@wangxiaoxuan273
resolve comments
1998baf 
Signed-off-by: Xiaoxuan Wang <[email protected]>
@wangxiaoxuan273
fix unit test
0f89333 
Signed-off-by: Xiaoxuan Wang <[email protected]>
@wangxiaoxuan273
fix tests
914a23c 
Signed-off-by: Xiaoxuan Wang <[email protected]>
@wangxiaoxuan273
fix failed test
7bd8ddd 
Signed-off-by: Xiaoxuan Wang <[email protected]>
@wangxiaoxuan273
fix test
cb59903 
Signed-off-by: Xiaoxuan Wang <[email protected]>
@wangxiaoxuan273
fix
bb42afd 
Signed-off-by: Xiaoxuan Wang <[email protected]>
@wangxiaoxuan273
switch
c33f577 
Signed-off-by: Xiaoxuan Wang <[email protected]>
@wangxiaoxuan273 wangxiaoxuan273 mentioned this pull request Apr 12, 2024
Closed
shizhMSFT
shizhMSFT approved these changes Apr 12, 2024
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

qweeah
qweeah approved these changes Apr 12, 2024
View reviewed changes
Copy link
Contributor

@qweeah qweeah left a comment

Choose a reason for hiding this comment

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

LGTM

@qweeah qweeah merged commit 61557e0 into oras-project:main Apr 12, 2024
8 checks passed
@shizhMSFT shizhMSFT mentioned this pull request Apr 12, 2024
Closed
3 tasks
@wangxiaoxuan273 wangxiaoxuan273 deleted the token branch April 15, 2024 02:58
FeynmanZhou pushed a commit to FeynmanZhou/oras that referenced this pull request May 11, 2024
@wangxiaoxuan273 @FeynmanZhou
feat: make --identity-token an alias of --password (oras-project#1294)
117d05b 
Signed-off-by: Xiaoxuan Wang <[email protected]>
@qweeah qweeah mentioned this pull request May 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@qweeah qweeah qweeah approved these changes

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

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

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

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

Successfully merging this pull request may close these issues.

Make --identity-token an alias of --password
4 participants
@wangxiaoxuan273 @qweeah @TerryHowe @shizhMSFT