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

Add support for pipeline #1

Merged
merged 21 commits into from Mar 13, 2017
Merged

Add support for pipeline #1

merged 21 commits into from Mar 13, 2017

Conversation

@enwi
Copy link

enwi commented Feb 7, 2017

This pull-request fixes the folowing issues

@rodrigc
Copy link

rodrigc commented Feb 15, 2017

Good stuff! I downloaded your branch and installed on my system which is Jenkins 2.46 with all the latest plugins. I tried to use the Pipeline Snippet Generator to show the pipeline commands.

However the snippet generator only showed:

step <object of type org.korosoft.jenkins.plugin.rtp.RichTextPublisher>
@enwi
Copy link
Author

enwi commented Feb 16, 2017

I've got Jenkins 2.32.2 and everything works fine, but I know that when you use the snippet generator and select "step: General Build Step" and then select "Publish rich text message" the generator will show step <object of type org.korosoft.jenkins.plugin.rtp.RichTextPublisher>, because I did not implement the functionality for "General Build Steps". On the other hand you can just select "rtp" as a sample step in the snippet generator and it should generate sth. like:
rtp parserName: 'WikiText', stableText: ''
Did you try that?
Also maybe I should revise the code to make it not to show up at all in "General Build Steps".

@rodrigc
Copy link

rodrigc commented Feb 16, 2017

Yes, you are absolutely correct.

  • when I selected rtp as the sample step, I saw rtp parserName: 'WikiText', stableText: ''
  • when I selected step: General Build Step and then Publish rich text message, then I saw <object of type org.korosoft.jenkins.plugin.rtp.RichTextPublisher>

I don't know what the convention is for implementing pipeline build steps, but
I think going in the direction to avoid confusion would be best.

@enwi
Copy link
Author

enwi commented Feb 16, 2017

Actually I just checked the code and also found this tutorial. So when you follow the tutorial the "General Build Step" should also work for rtp. I changed some code arround and will test if it will work now.

@enwi
Copy link
Author

enwi commented Feb 16, 2017

So I did a little bit of research and it seems that in "DescriptorImpl" of "RichTextPublisher" in function "FormValidation"
AbstractProject
needs to be replaced by
Job
like they did it. As well as
FilePath workspace = project.getSomeWorkspace();
by
FilePath workspace = new FilePath(project.getBuildDir());
also the null check can be removed. But this does not work for me at all.

…ed as a "General Build Step"
@enwi
Copy link
Author

enwi commented Feb 16, 2017

@rodrigc I fixed the snippet generator problem with this commit. Now you have the option to use rtp directly as a step or as a metastep ("General Build Step"), but I would always prefer to use it directly.

if (!useLastStable) {
return getRichTextFromActions(project.getActions(AbstractRichTextAction.class));
} else {
for (AbstractBuild<?, ?> abstractBuild : project.getBuilds()) {
for (Run<?, ?> build : project.getBuilds()) {

This comment has been minimized.

Copy link
@masterhard

masterhard Feb 23, 2017

Member

run would be a better loop variable name here


Result res = abstractBuild.getResult();
Result res = build.getResult();
if(res == null) {
throw new IllegalStateException("abstractBuild.getResult() is null!");

This comment has been minimized.

Copy link
@masterhard

masterhard Feb 23, 2017

Member

Exception message should be updated too

@enwi
Copy link
Author

enwi commented Feb 24, 2017

@masterhard I committed your suggested changes. Let me know if there is anything else that needs to be changed. I also set the regarding issues in jira to "in review".

@enwi
Copy link
Author

enwi commented Mar 3, 2017

@masterhard Just out of curiosity when will you merge this PR ?

@lechen26
Copy link

lechen26 commented Mar 13, 2017

when this will be added to the pipeline?

@masterhard masterhard merged commit 86ce8d8 into jenkinsci:master Mar 13, 2017
1 check passed
1 check passed
Jenkins This pull request looks good
Details
@glib-briia
Copy link

glib-briia commented Apr 21, 2017

Hi, Anyone got any ideas when is it going to be released?

@masterhard
Copy link
Member

masterhard commented Apr 27, 2017

I'm on it. Waiting for jenkins-infra/repository-permissions-updater#291 approval now.

@masterhard
Copy link
Member

masterhard commented Apr 28, 2017

Done

@enwi enwi deleted the enwi:recovery branch Aug 11, 2017
@renfeng

This comment has been minimized.

Copy link

renfeng commented on pom.xml in be8a7de Nov 15, 2018

Not obvious why it requires pipeline. What if someone don't want pipeline?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.