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

[JENKINS-45845] Change containerCap and instanceCap 0 to mean do not use #199

Merged
merged 3 commits into from Aug 25, 2017

Conversation

Projects
None yet
2 participants
@carlossg

carlossg commented Aug 16, 2017

With current behavior

It is inconsistent, probably needs to be fixed https://issues.jenkins-ci.org/browse/JENKINS-45845

@carlossg

This comment has been minimized.

Show comment
Hide comment
@carlossg

carlossg Aug 16, 2017

Seems that a instanceCap=0 should be valid, meaning "I don't want to use this template", it may require some changes

ping @marvinthepa

carlossg commented Aug 16, 2017

Seems that a instanceCap=0 should be valid, meaning "I don't want to use this template", it may require some changes

ping @marvinthepa

@marvinthepa

This comment has been minimized.

Show comment
Hide comment
@marvinthepa

marvinthepa commented Aug 16, 2017

😃

@carlossg

This comment has been minimized.

Show comment
Hide comment
@carlossg

carlossg Aug 16, 2017

Added a possible implementation

carlossg commented Aug 16, 2017

Added a possible implementation

@@ -216,7 +216,7 @@ public String getRemoteFs() {
}
public void setInstanceCap(int instanceCap) {
if (instanceCap <= 0) {
if (instanceCap < 0) {

This comment has been minimized.

@marvinthepa

marvinthepa Aug 16, 2017

Maybe I am wrong, but doesn't this also need to go into readResolve for backward compatibility?

@marvinthepa

marvinthepa Aug 16, 2017

Maybe I am wrong, but doesn't this also need to go into readResolve for backward compatibility?

This comment has been minimized.

@carlossg

carlossg Aug 16, 2017

if you had instanceCap=0 the xml is written with <instanceCap>2147483647</instanceCap> so if I'm not mistaken there is nothing to do

@carlossg

carlossg Aug 16, 2017

if you had instanceCap=0 the xml is written with <instanceCap>2147483647</instanceCap> so if I'm not mistaken there is nothing to do

This comment has been minimized.

@marvinthepa

marvinthepa Aug 17, 2017

Sounds good.
I thought there might be a situation where the xml is loaded with instanceCap not set at all, but if that cannot occur I guess it's fine.

@marvinthepa

marvinthepa Aug 17, 2017

Sounds good.
I thought there might be a situation where the xml is loaded with instanceCap not set at all, but if that cannot occur I guess it's fine.

@carlossg carlossg changed the title from [JENKINS-45845] Document containerCap and instanceCap to [JENKINS-45845] Change containerCap and instanceCap 0 to mean do not use Aug 17, 2017

@carlossg carlossg merged commit c71a20f into master Aug 25, 2017

1 check passed

continuous-integration/jenkins/branch This commit looks good
Details

@carlossg carlossg deleted the JENKINS-45845 branch Aug 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment