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.

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-31967] Specifying decimal numbers for JUnit health factor in Pipeline snippets results in invalid code #16

Merged

Conversation

Projects
None yet
4 participants
@jglick
Copy link
Member

left a comment

Minor issues but looks right for the most part.

@@ -707,4 +706,22 @@ private Object readResolve() {
private static final Logger LOGGER = Logger.getLogger(DescribableModel.class.getName());

private static final long serialVersionUID = 1L;

/**
* Given the primitive type, returns the VM default value for that type in a boxed form.

This comment has been minimized.

Copy link
@jglick

jglick Apr 24, 2017

Member

Please file a PR for Jenkins core that fixes the bug in ReflectionUtils, and add a TODO comment here linking to that so in the future we could switch back to the standard version.

this.doubleValue1 = doubleValue1;
}

@DataBoundSetter public void setBooleanValue2(boolean booleanValue2) {

This comment has been minimized.

Copy link
@jglick

jglick Apr 24, 2017

Member

This does not make sense. A given field may either be mentioned in a @DataBoundConstructor, or a @DataBoundSetter, but not both.

}
}

@Test

This comment has been minimized.

Copy link
@jglick

jglick Apr 24, 2017

Member

@Issue

"doubleValue2", Double.MAX_VALUE
));

assertEquals(Boolean.TRUE, instance.isBooleanValue1());

This comment has been minimized.

Copy link
@jglick

jglick Apr 24, 2017

Member

Probably more easily done with the roundTrip helper method.

artem-fedorov added some commits Apr 27, 2017

@abayer

abayer approved these changes May 22, 2017

Copy link
Member

left a comment

Looks good to me but would like @jglick to give it another look, obviously.

@oleg-nenashev
Copy link
Member

left a comment

Looks good 👍

@jglick jglick self-requested a review May 22, 2017

@jglick
Copy link
Member

left a comment

Unnecessary public addition, otherwise looks fine.

/**
* Given the primitive type, returns the VM default value for that type in a boxed form.
*/
public static Object getVmDefaultValueForPrimitiveType(Class<?> type) {

This comment has been minimized.

Copy link
@jglick

jglick May 22, 2017

Member

This should be private.

@jglick

jglick approved these changes May 23, 2017

@jglick jglick merged commit 1485eca into jenkinsci:master May 25, 2017

1 check passed

Jenkins This pull request looks good
Details

jglick added a commit that referenced this pull request May 25, 2017

Merged #16: [JENKINS-31967] Specifying decimal numbers for JUnit heal…
…th factor in Pipeline snippets results in invalid code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.