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

[JENKINS-54133] Encode console notes on master for JEP-210 compatibility #32

Merged
merged 6 commits into from
Oct 29, 2018
Merged

[JENKINS-54133] Encode console notes on master for JEP-210 compatibility #32

merged 6 commits into from
Oct 29, 2018

Conversation

jglick
Copy link
Member

@jglick jglick commented Oct 18, 2018 •
edited
Loading

JENKINS-54133: As opposed to the general case in jenkinsci/workflow-durable-task-step-plugin#80, in this case it is easy enough to be compatible with JEP-210. Test deps are on jenkinsci/durable-task-plugin#86 + jenkinsci/workflow-durable-task-step-plugin#87.

@jglick
Copy link
Member Author

jglick commented Oct 18, 2018

There is a timeout in AntWrapperTest.durability which I can reproduce locally but do not yet understand. It does not appear to be caused by the use of watch mode. Apparently there are two similar WorkflowRuns after the restart, perhaps indicating yet another bug in AbstractLazyLoadRunMap?

@jglick jglick mentioned this pull request Oct 19, 2018
Merged
@jglick
Copy link
Member Author

jglick commented Oct 19, 2018 •
edited
Loading

Weirdness. With workflow-durable-task-step 2.16, the test passes. With 2.18, it fails. With 2.17, it passes, but without clearly wrong things happening, which I blame on jenkinsci/workflow-durable-task-step-plugin#53. The failures in 2.18 seem to be due to jenkinsci/durable-task-plugin#49 and yet, with jenkinsci/workflow-durable-task-step-plugin#53 reverted by jenkinsci/workflow-durable-task-step-plugin#81, the test can pass even with the current durable-task so long as watch mode (jenkinsci/durable-task-plugin#60) is disabled. I do not really know what is going on yet.

@jglick jglick mentioned this pull request Oct 19, 2018
Merged
@dwnusbaum dwnusbaum mentioned this pull request Oct 26, 2018
Merged
dwnusbaum
dwnusbaum approved these changes Oct 26, 2018
View reviewed changes
Copy link
Member

@dwnusbaum dwnusbaum left a comment

Choose a reason for hiding this comment

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

The fix here looks good to me, although I think it would be good to update related documentation as described in jenkinsci/workflow-durable-task-step-plugin#80 (comment).

pom.xml
<jenkins.version>1.651.3</jenkins.version>
<revision>1.9</revision>
<changelist>-SNAPSHOT</changelist>
<jenkins.version>2.121.2</jenkins.version>
Copy link
Member

Choose a reason for hiding this comment

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

Big jump, but required for JEP-210 compatibility. Based on these usage stats, around 73% of users of ant 1.8 are on Jenkins 2.121.2 or newer anyway, so this seems fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Anyway even if there is some unrelated fix we realize we need to deliver urgently to the remaining 27%, it is easy enough to publish a backport.

@jglick jglick merged commit d7dd61e into jenkinsci:master Oct 29, 2018
@jglick jglick deleted the JEP-210-tests branch October 29, 2018 22:29
@jglick jglick mentioned this pull request Oct 29, 2018
Merged
Wadeck added a commit to Wadeck/ant-plugin that referenced this pull request Oct 30, 2018
@Wadeck
Does not seem required due to jenkinsci#32 build, but still fail loca…
baad36d 
…lly.
@Wadeck Wadeck mentioned this pull request Oct 30, 2018
Merged
@jglick jglick mentioned this pull request Oct 30, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jvz jvz jvz approved these changes

@dwnusbaum dwnusbaum dwnusbaum approved these changes

@svanoort svanoort Awaiting requested review from svanoort

@abayer abayer Awaiting requested review from abayer

@rsandell rsandell Awaiting requested review from rsandell

@joseblas joseblas Awaiting requested review from joseblas

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

Successfully merging this pull request may close these issues.
3 participants
@jglick @jvz @dwnusbaum