Skip to content
This repository has been archived by the owner on Nov 5, 2022. It is now read-only.

[JENKINS-26481] Generate CpsDefaultGroovyMethods, invoke using GroovyCategorySupport #56

Merged
merged 30 commits into from
May 30, 2017

Conversation

jglick
Copy link
Member

@jglick jglick commented May 16, 2017

I accidentally merged #54 and it seems GitHub offers no way to unmerge a PR.

Pulls the category trick down from jenkinsci/workflow-cps-plugin#124 (and should become upstream of that if all works out).

Implements #7, picking up artifacts from cloudbees/groovy-cps-dgm-builder#4 (ideally this would be automated by Maven but for now it is manual).

Supersedes #52 (though the expanded tests could be sideported).

@reviewbybees

@abayer
Copy link
Contributor

abayer commented May 16, 2017

I was wondering what happened there...

@jglick jglick changed the title [JENKINS-26481] Working on supporting CPS-transformed closures passed to DefaultGroovyMethods [JENKINS-26481] Generate CpsDefaultGroovyMethods, invoke using GroovyCategorySupport May 17, 2017
Copy link
Contributor

@abayer abayer left a comment

Choose a reason for hiding this comment

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

So I think we need sort - it's too obvious a method to use once we've got each, collect, etc... I know it's some work, but I think of the things that are not yet translated, that's the most important.

I'm also not sure whether we should hold this until StringGroovyMethods is ready too - I think we can probably wait on it, but it feels like we might as well do it now while we're here?

@jglick
Copy link
Member Author

jglick commented May 23, 2017

Well I do not know how to implement sort so my feeling is that if @kohsuke wants to take it on some time, great, else we do not do it. (It would have been easier had the translation tool been written to reuse CpsTransformer, treating its Java input as Groovy code except in special cases like new arrays with initializers, but instead it is a separate beast, and quite hard to work on.)

W.r.t. StringGroovyMethods, if you feel like writing a test for it now, I should be able to include it pretty easily (if all goes well), but I am guessing it is rarely used so I was not going to hold things up for that.

@abayer
Copy link
Contributor

abayer commented May 23, 2017

Ok, sounds reasonable on both counts.

@jglick
Copy link
Member Author

jglick commented May 24, 2017

@reviewbybees done

@jglick jglick merged commit a25f920 into cloudbees:master May 30, 2017
@jglick jglick deleted the closures-JENKINS-26481 branch May 30, 2017 21:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants