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

Updated to use SimpleBuildStep and thus be usable from a Workflow build #2

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@jglick
Member

jglick commented Aug 12, 2015

node {
  catchError {
    if (Integer.parseInt(env.BUILD_NUMBER) % 3 == 0) {
      error 'oops'
    }
  }
  step([$class: 'EmotionalJenkinsPublisher'])
}

Currently kind of useless since in 1.580.1 the butler sidepanel is no longer displayed, but that could be fixed independently I suppose. Functionality confirmed via inspection of EmotionalJenkinsAction.result via Job.allActions and Run.allActions.

@reviewbybees

@reviewbybees

This comment has been minimized.

Show comment
Hide comment
@reviewbybees

reviewbybees Aug 12, 2015

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

reviewbybees commented Aug 12, 2015

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@@ -3,7 +3,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>1.572</version><!-- which version of Jenkins is this plugin built against? -->
<version>1.580.1</version><!-- which version of Jenkins is this plugin built against? -->

This comment has been minimized.

@oleg-nenashev

oleg-nenashev Aug 12, 2015

Member

A fortunate case when we don't need to justify it

@oleg-nenashev

oleg-nenashev Aug 12, 2015

Member

A fortunate case when we don't need to justify it

Show outdated Hide outdated ...java/org/jenkinsci/plugins/emotional_jenkins/EmotionalJenkinsAction.java
@@ -7,8 +7,6 @@
private Result result;
public EmotionalJenkinsAction() {}

This comment has been minimized.

@oleg-nenashev

oleg-nenashev Aug 12, 2015

Member

Uh? This constructor seems to be used for "no result" purposes. It's deletion breaks the binary compatibility

@oleg-nenashev

oleg-nenashev Aug 12, 2015

Member

Uh? This constructor seems to be used for "no result" purposes. It's deletion breaks the binary compatibility

This comment has been minimized.

@jglick

jglick Aug 12, 2015

Member

seems to be used for "no result" purposes

Not any more, see LastBuildActionFactory.

@jglick

jglick Aug 12, 2015

Member

seems to be used for "no result" purposes

Not any more, see LastBuildActionFactory.

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Aug 12, 2015

Member

🐛 due to the constructor deletion

Member

oleg-nenashev commented Aug 12, 2015

🐛 due to the constructor deletion

@amuniz

This comment has been minimized.

Show comment
Hide comment
@amuniz

amuniz commented Aug 12, 2015

🐝

@jenkinsadmin

This comment has been minimized.

Show comment
Hide comment
@jenkinsadmin

jenkinsadmin Aug 12, 2015

Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

Member

jenkinsadmin commented Aug 12, 2015

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Aug 12, 2015

Member

I still think it would be better to keep the constructor and just deprecate it.

Member

oleg-nenashev commented Aug 12, 2015

I still think it would be better to keep the constructor and just deprecate it.

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev
Member

oleg-nenashev commented Aug 12, 2015

🐝

@jglick

This comment has been minimized.

Show comment
Hide comment
@jglick
Member

jglick commented Aug 12, 2015

@reviewbybees

This comment has been minimized.

Show comment
Hide comment
@reviewbybees

reviewbybees Aug 13, 2015

This pull request has completed our internal processes and we now respectfully request the maintainers of this repository to consider our proposal contained within this pull request for merging.

reviewbybees commented Aug 13, 2015

This pull request has completed our internal processes and we now respectfully request the maintainers of this repository to consider our proposal contained within this pull request for merging.

@jglick

This comment has been minimized.

Show comment
Hide comment
@jglick
Member

jglick commented Aug 21, 2015

@kiy0taka FYI

@jglick

This comment has been minimized.

Show comment
Hide comment
@jglick
Member

jglick commented Oct 7, 2015

public EmotionalJenkinsPublisher() {}
@Override
public void perform(Run<?, ?> build, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException {

This comment has been minimized.

@KostyaSha

KostyaSha Oct 26, 2015

Member

> 120 width

@KostyaSha

KostyaSha Oct 26, 2015

Member

> 120 width

@Override
public BuildStepDescriptor<Publisher> getDescriptor() {
return DESCRIPTOR;
@Extension public static final class LastBuildActionFactory extends TransientActionFactory<Job> {

This comment has been minimized.

@KostyaSha

KostyaSha Oct 26, 2015

Member

Code style mess, this file contains normal styling with annotation on new line, please follow it.

@KostyaSha

KostyaSha Oct 26, 2015

Member

Code style mess, this file contains normal styling with annotation on new line, please follow it.

return Job.class;
}
@Override public Collection<? extends Action> createFor(Job j) {

This comment has been minimized.

@KostyaSha

KostyaSha Oct 26, 2015

Member

The same

@KostyaSha

KostyaSha Oct 26, 2015

Member

The same

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