Skip to content

[JENKINS-52724/JENKINS-52737] Inherit properties from the local docker config.json#72

Merged
oleg-nenashev merged 2 commits into
jenkinsci:masterfrom
kvalev:proxy
May 13, 2019
Merged

[JENKINS-52724/JENKINS-52737] Inherit properties from the local docker config.json#72
oleg-nenashev merged 2 commits into
jenkinsci:masterfrom
kvalev:proxy

Conversation

@kvalev

@kvalev kvalev commented Jul 30, 2018

Copy link
Copy Markdown
Contributor

When creating a new config folder, try to copy some of the properties of the existing config.json file.

Fixes JENKINS-52724 and JENKINS-52737.

@kvalev

kvalev commented Aug 6, 2018

Copy link
Copy Markdown
Contributor Author

@jglick Can you please take a look at this PR.

@jglick
jglick requested a review from dwnusbaum April 9, 2019 17:27
@jglick

jglick commented Apr 9, 2019

Copy link
Copy Markdown
Member

Probably right? Did not look closely.

@jglick

jglick commented Apr 9, 2019

Copy link
Copy Markdown
Member

Amends #67 IIUC.

@oleg-nenashev
oleg-nenashev self-requested a review April 29, 2019 14:39
@oleg-nenashev

Copy link
Copy Markdown
Member

@dwnusbaum did you have a chance to review it?

@dwnusbaum

Copy link
Copy Markdown
Member

@oleg-nenashev No, the main patch seems fine at a glance but I'd want to look through Docker docs to understand what exactly can be in the file to see if anything else should be blacklisted before approving.

@oleg-nenashev oleg-nenashev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks OK from my PoV. There are some edge cases with file handling, but the logic is good enough

@oleg-nenashev oleg-nenashev changed the title Inherit properties from the local docker config.json [JENKINS-52724/JENKINS-52737] Inherit properties from the local docker config.json May 13, 2019
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