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

Evaluate settings.xml later, and per id #1712

Merged
merged 6 commits into from
May 15, 2019
Merged

Evaluate settings.xml later, and per id #1712

merged 6 commits into from
May 15, 2019

Conversation

loosebazooka
Copy link
Member

@loosebazooka loosebazooka commented May 10, 2019 •
edited
Loading

  • Also adds in SettingsFixture for tests that want to
    use an actual instance of settings.xml and optionally
    can create a SettingsDecrypter from a settings-security.xml
  • The tests should be much more concrete now.
  • Function<String, Optional<AuthProvider>> was turned in an interface InferredAuth, so we could handle exceptions better.

fixes #1709

@loosebazooka
Evaluate settings.xml later, and per id
d4d331e 
- Also adds in SettingsFixture for tests that want to
  use an actual instance of settings.xml and optionally
  can create a SettingsDecrypter from a settings-security.xml
@loosebazooka loosebazooka force-pushed the evaluateMavenSettingsLater branch from 24917e3 to d4d331e Compare May 10, 2019 21:56
chanseokoh
chanseokoh reviewed May 10, 2019
View reviewed changes
Copy link
Member

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

I'll go over this again next week, but here's what I've found after taking a quick look.

...ins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/InferredAuthProvider.java Show resolved Hide resolved
...ins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/InferredAuthProvider.java Show resolved Hide resolved
...ns-common/src/main/java/com/google/cloud/tools/jib/plugins/common/InferredAuthException.java Outdated Show resolved Hide resolved
jib-maven-plugin/src/test/resources/maven/settings/settings-security.xml Show resolved Hide resolved
jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProxyProvider.java Outdated Show resolved Hide resolved
jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProxyProvider.java Outdated Show resolved Hide resolved
jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProxyProvider.java Show resolved Hide resolved
jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProxyProvider.java Outdated Show resolved Hide resolved
chanseokoh
chanseokoh reviewed May 13, 2019
View reviewed changes
Copy link
Member

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

One fix, and LGTM.

jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/MavenSettingsProxyProvider.java Outdated Show resolved Hide resolved
@chanseokoh chanseokoh mentioned this pull request May 15, 2019
Closed
@loosebazooka loosebazooka merged commit d3b1e8c into master May 15, 2019
@chanseokoh chanseokoh deleted the evaluateMavenSettingsLater branch June 7, 2019 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chanseokoh chanseokoh chanseokoh approved these changes

@briandealwis briandealwis Awaiting requested review from briandealwis

@TadCordle TadCordle Awaiting requested review from TadCordle

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
3 participants
@loosebazooka @chanseokoh @googlebot