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 Jenkins pipeline support in ircbot-plugin - an ircNotify() step #21

Merged
merged 19 commits into from
Aug 6, 2019

Conversation

jimklimov
Copy link
Contributor

@jimklimov jimklimov commented Jul 22, 2019

Status: "Works for us, review and merge welcome" ;)

I wanted to post this separately from the style cleanup done in #20, but it was too complicated to superficially make incompatible patch sets just for the sake of keeping PRs separate. They are in different commits and I hope this suffices to keep the review chores relatively simple.

Until the #20 is merged, reviewers are welcome to compare the PR'ed branch contents added on top of that cleanup via jimklimov/ircbot-plugin@fix-style...jimklimov:pipeline-support

Feature-wise, this builds upon and requires jenkinsci/instant-messaging-plugin#16 which implements the pipeline support (note semi-cosmetic fixes proposed in akomakom/instant-messaging-plugin#1 not merged so far), and replicates the consumer of that support originally done for the Jabber plugin in jenkinsci/jabber-plugin#18 with a slight adaptation for the ircbot plugin.

Note that architecturally here the pipeline Step interface is provided by the protocol plugins (irc, jabber) to send notifications over this or that medium, but the real bulk of implementation is in the commit instant-messaging plugin.

When it hopefully gets to merging this PR ecosystem, please take care to bump the published releases and fix the version requirements in the pom.xml files, so sufficiently advanced dependencies get used by maven and the Jenkins plugin installer.

Unfortunately, not being much of a Java developer, I did not find any better way to build the wad of plugins than to mvn clean package install the codebase which includes jenkinsci/instant-messaging-plugin#16 and a modified pom.xml with <version>1.36-SNAPSHOT-PIPELINES</version> so I had the specially tagged JAR in my local maven repository, and then I could build, test and use the ircbot codebase which requires this tagged version. This would probably preclude automated Jenkins builds of the PR, however (and would be an additional reason to need the post-merge changes outlined above).

@jimklimov
Copy link
Contributor Author

jimklimov commented Jul 22, 2019

CC @Casz @kutzi @akomakom @olivergondza

@jimklimov jimklimov changed the title Pipeline support Add Jenkins pipeline support in ircbot-plugin - an ircNotify() step Jul 22, 2019
@jetersen
Copy link
Member

Not sure why I am cced 😕

@kutzi
Copy link
Member

kutzi commented Jul 22, 2019

@jimklimov I'm still listed as maintainer for this plugin, but I'm not really acting anymore - sorry.
As I'm not part of the jenkins-dev mailing list anymore, I cannot even ask to remove me as maintainer from the wiki page ...

@kutzi
Copy link
Member

kutzi commented Jul 22, 2019

That being said: I think you'll have the best chances if you ask on the jenkins-dev mailing list for support.
Maybe someone else wants to step up as the maintainer? Maybe you want to?

@jimklimov
Copy link
Contributor Author

@Casz : sorry, a miss on my side

You were very helpful in other reviews on other components, and I must have mixed up which developments you were part of. Still, every day for us something new, to dig in, right? ;)

@jimklimov
Copy link
Contributor Author

Alas, the CI tests would be red until the jenkinsci/instant-messaging-plugin#16 (and follow-up, see above) is merged.

@@ -56,6 +59,10 @@
private BuildToChatNotifier buildToChatNotifier = new DefaultBuildToChatNotifier();
private MatrixJobMultiplier matrixNotifier = MatrixJobMultiplier.ONLY_PARENT;

// Instead of build status messages, send an arbitrary message to specified
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: As subsequent co-evolution of ircbot and instant-messaging plugins, maybe makes sense to evict this feature there, similarly to extraMessage done in @akomakom's branch.

@jimklimov
Copy link
Contributor Author

At this time, codebase of this PR works against the codebase in release 1.37 of the core instant-messaging plugin.

While it successfully adds the customMessage support to the IRC notification (pipeline ircNotify at least, as tested), the solution is not quite streamlined (and also the extraMessage support is lacking). These will be improved in a next PR that would depend on a newer revision (eventually release) of the core instant-messaging plugin.

Merging now to freeze and publish a working improvement, before I break it again for a next iteration ;)

@jimklimov jimklimov merged commit fef99b4 into jenkinsci:master Aug 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants