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

Move reusable options to a dedicated package #386

Merged
merged 18 commits into from
May 19, 2022
Merged

Move reusable options to a dedicated package #386

merged 18 commits into from
May 19, 2022

Conversation

qweeah
Copy link
Contributor

@qweeah qweeah commented May 16, 2022 •
edited
Loading

This PR

  1. Adds a bug fix for empty ca file path option, as well as a unit test for it.
  2. Adds a package which groups CMD options used in different senarios.

Resolves #385

@qweeah
Fix login bug for empty ca file
acca895 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Move user options to dedicated package
8cc26cc 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Code clean
61a6083 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah qweeah marked this pull request as draft May 17, 2022 01:42
@qweeah
Refactoring
d2937ae 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Generalize process of applying cobra flags
c9db85d 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah qweeah marked this pull request as ready for review May 17, 2022 04:15
@qweeah qweeah changed the title Move reusable options to a common file Move reusable options to a dedicated package May 17, 2022
@qweeah
Move option package into cmd
6c9a8fe 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
shizhMSFT
shizhMSFT reviewed May 17, 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.

It would be better if all exported methods / types are documented, even if they are internal.

internal/http/certificate.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/applier.go Show resolved Hide resolved
cmd/oras/login.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/credential.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/credential.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/tls.go Outdated Show resolved Hide resolved
cmd/oras/login.go Show resolved Hide resolved
cmd/oras/login.go Outdated Show resolved Hide resolved
cmd/oras/login.go Outdated Show resolved Hide resolved
@qweeah
Resolve comments
cbf7f98 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Code clean
a2ea274 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Code clean
4d419b9 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Add document
f18f6ba 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Merge remote-tracking branch 'origin_src/main' into common-opts
e9097e7
@qweeah
Refactor flag applier
fc08f88 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
shizhMSFT
shizhMSFT reviewed May 18, 2022
View reviewed changes
cmd/oras/internal/option/applier.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/applier.go Show resolved Hide resolved
cmd/oras/login.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/tls.go Outdated Show resolved Hide resolved
@qweeah
Clean comment
ee278d7 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Nit: add space after license
60d81aa 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Refactor
6e52215 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Refactor
535fef7 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah qweeah requested a review from shizhMSFT May 19, 2022 04:45
shizhMSFT
shizhMSFT reviewed May 19, 2022
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_test.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/remote_test.go Outdated Show resolved Hide resolved
cmd/oras/login.go Outdated Show resolved Hide resolved
@qweeah
Resolve comments
3dd6e76 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Code clean
9525643 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
shizhMSFT
shizhMSFT approved these changes May 19, 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 db86d1c into oras-project:main May 19, 2022
@qweeah qweeah deleted the common-opts branch August 25, 2023 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sajayantony sajayantony sajayantony left review comments

@shizhMSFT shizhMSFT shizhMSFT approved these changes

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

Successfully merging this pull request may close these issues.

Move common options to a common file
3 participants
@qweeah @sajayantony @shizhMSFT