-
Notifications
You must be signed in to change notification settings - Fork 42
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
Conversation
…t globally configured channels)
…s alternative to build results reporting
…less step does the common default activity
…SHOT-PIPELINES version specifically
Not sure why I am cced 😕 |
@jimklimov I'm still listed as maintainer for this plugin, but I'm not really acting anymore - sorry. |
That being said: I think you'll have the best chances if you ask on the jenkins-dev mailing list for support. |
@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? ;) |
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 |
There was a problem hiding this comment.
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.
…om Jabber implem) and move into steps/
…cution subclass too
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 Merging now to freeze and publish a working improvement, before I break it again for a next iteration ;) |
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 modifiedpom.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).