[FIXED JENKINS-41811] Expose event origin to listeners #164

stephenc · 2017-02-07T15:26:11Z

Downstream of jenkinsci/scm-api-plugin#27

Remove WIP label once [FIXED JENKINS-41795] Add field to track event origins scm-api-plugin#27 merged and released and this PR updated to release version

Upstream of changes in GitHub Branch Source to start exposing the event origin

See also jenkinsci/branch-api-plugin#79 (includes screenshots of events from GitHub Branch Source with this PRs changes integrated)

@reviewbybees

This change is

stephenc · 2017-02-07T15:28:12Z

pom.xml

@@ -99,7 +99,13 @@
        <dependency>
            <groupId>org.jenkins-ci.plugins</groupId>
            <artifactId>git</artifactId>
-            <version>2.4.0</version>
+            <version>2.4.0</version> <!-- should really be 2.6.4 to match scm-api but ok as git wass forward compat -->


Workflow tests need rework for the updated git dependency, but as git plugin didn't break the rules of SCM API we don't need to update to 2.6.4 / 3.0.4... just would be missing the SCMEvent integration in git plugin, but the tests in this plugin do not rely on that functionality, so no harm

stephenc · 2017-02-07T15:32:57Z

src/main/java/com/cloudbees/jenkins/GitHubWebHook.java

     */
+    @Deprecated
+    @Restricted(NoExternalUse.class)


From what I can see the only consumer of this API is the 1.x GitHub Branch Source. If there are other use cases for consumers remaining post SCM API 2.0.x updates then this would need to be augmented to pass through the origin.

I'd much rather kill off the code path, so by adding the @Restricted we will flush out any impls without breaking them until they update their dependency

@jglick that single impl is not in GHBS 2.0.x

KostyaSha · 2017-02-07T15:43:57Z

pom.xml

+        <dependency>
+            <groupId>org.jenkins-ci.plugins</groupId>
+            <artifactId>scm-api</artifactId>
+            <version>2.0.3-SNAPSHOT</version>


👎 on scm-api dependency

Why? scm-api is harmless—it has no UI.

Because it adds useless dependency that participate in multibranch/branch api chaos with cycle dependencies and huge pages with instructions what to do with broken instances. I also have gh plugins depending on github-plugin and i don't want to deal with redundant dependencies. github-plugin as trigger and eventing mechanism may work without additional deps. Freely add Listener calls instead of deprecating it. I would also like to see firstly such changes in gitlab and other trigger plugins, not only gh.

@KostyaSha you already have the dependency via git 2.4.0 (but just an older version) and we need the SCMEvent.originOf(req) helper method

KostyaSha · 2017-02-07T15:47:31Z

src/main/java/com/cloudbees/jenkins/GitHubTrigger.java

@@ -24,6 +24,8 @@
    // TODO: document me
    void onPost(String triggeredByUser);

+    void onPost(String origin, String triggeredByUser);


should wait until java8

Could instead introduce an extension interface.

KostyaSha

From what i see i'm against depending on scm-api that adds mess in plugins with all this new plugins.

KostyaSha · 2017-02-07T15:50:53Z

src/main/java/com/cloudbees/jenkins/GitHubWebHook.java

@@ -153,7 +157,10 @@ public static Jenkins getJenkinsInstance() throws IllegalStateException {
     * Other plugins may be interested in listening for these updates.
     *
     * @since 1.8
+     * @deprecated we do not think this API is required any more, if we are wrong, please raise a JIRA


jglick

Not binary compatible.

jglick · 2017-02-07T16:05:18Z

pom.xml

@@ -99,7 +99,13 @@
        <dependency>
            <groupId>org.jenkins-ci.plugins</groupId>
            <artifactId>git</artifactId>
-            <version>2.4.0</version>
+            <version>2.4.0</version> <!-- should really be 2.6.4 to match scm-api but ok as git wass forward compat -->


jglick · 2017-02-07T16:05:51Z

pom.xml

+        <dependency>
+            <groupId>org.jenkins-ci.plugins</groupId>
+            <artifactId>scm-api</artifactId>
+            <version>2.0.3-SNAPSHOT</version>


preferably specify a timestamp

jglick · 2017-02-07T16:06:45Z

src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java

@@ -87,6 +96,7 @@ private boolean runPolling() {
                        PrintStream logger = listener.getLogger();
                        long start = System.currentTimeMillis();
                        logger.println("Started on " + DateFormat.getDateTimeInstance().format(new Date()));
+                        logger.println("Started by event from " + origin);


Can origin be null? Or would it be "?" here?

jglick · 2017-02-07T16:07:21Z

src/main/java/com/cloudbees/jenkins/GitHubTrigger.java

@@ -24,6 +24,8 @@
    // TODO: document me
    void onPost(String triggeredByUser);

+    void onPost(String origin, String triggeredByUser);


Could instead introduce an extension interface.

jglick · 2017-02-07T16:08:55Z

src/main/java/com/cloudbees/jenkins/GitHubWebHook.java

     */
+    @Deprecated
+    @Restricted(NoExternalUse.class)


jglick · 2017-02-07T16:10:39Z

src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java

@@ -92,7 +93,7 @@ public void run() {
                            if (GitHubRepositoryNameContributor.parseAssociatedNames(job)
                                    .contains(changedRepository)) {
                                LOGGER.info("Poked {}", fullDisplayName);
-                                trigger.onPost(pusherName);
+                                trigger.onPost(origin, pusherName);


Must at least catch AbstractMethodError here.

stephenc · 2017-02-07T17:02:15Z

@KostyaSha I am not adding a dependency on scm-api, the current transitive dependency on scm-api is

    <dependency>
      <groupId>org.jenkins-ci.plugins</groupId>
      <artifactId>scm-api</artifactId>
      <version>0.2</version>
    </dependency>

KostyaSha · 2017-02-07T17:35:36Z

@KostyaSha I am not adding a dependency on scm-api, the current transitive dependency on scm-api is

That's terrible.

stephenc · 2017-02-07T18:43:08Z

@KostyaSha

That's terrible.

exactly, that version is so ancient which is why we need to bump it ;-)

KostyaSha · 2017-02-07T18:47:54Z

typed huge sentence on how i disappointed with jenkins & CB and deleted.

jglick · 2017-02-07T18:59:57Z

src/main/java/com/cloudbees/jenkins/GitHubTrigger.java

@@ -15,6 +15,7 @@
 * and triggers a build.
 *
 * @author aaronwalker
+ * @deprecated extend {@link GitHubTrigger2} instead


Careful—does this produce deprecation warnings for implementers of GitHubTrigger2?

Who has implemented it? it is doubtful the interface is even required.

Doesn't show as deprecated in my IDE

i think @jglick mean compiler warnings, but i guess they work only on annotations.

I wonder if this interface is required for https://github.com/jenkinsci/github-sqs-plugin/blob/master/src/main/java/com/base2services/jenkins/SqsBuildTrigger.java or if it is a gratuitous linkage between the two plugins. It is the only OSS consumer of this interface... the only reason for it to exist is so that the two trigger options will both work for triggering builds... which seems a really crappy UX

i.e. is it obvious to the user that checking one or both of the above check boxes will do exactly the same thing?

And those check boxes are kind of against the idiomatic Jenkins way for configuring hooks, i.e. through Poll SCM

I really think all this needs a good tidy up... but I am not going to take that on in this PR ;-)

(FTR the "Build when a message is published to an SQS Queue" sounds more that a bit scary... what kind of message and what queue? Does it mean that every time any message is pushed to any SQS Queue anywhere in the world that my build will be triggered?)

jglick · 2017-02-07T19:00:47Z

src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java

@@ -4,6 +4,7 @@
 import com.cloudbees.jenkins.GitHubRepositoryName;
 import com.cloudbees.jenkins.GitHubRepositoryNameContributor;
 import com.cloudbees.jenkins.GitHubTrigger;
+import com.cloudbees.jenkins.GitHubTrigger2;


Unused import AFAICT.

KostyaSha · 2017-02-07T19:14:48Z

So just to show useless information in UI you are adding useless hard dependencies, deprecating and hacking everything around? That will all end that nobody outside CB will be able to consume and implement plugins based on such code. For example credentials-plugin - nobody knows how to use it.

stephenc · 2017-02-07T19:17:14Z

@KostyaSha

typed huge sentence on how i disappointed with jenkins & CB and deleted.

Well writing out your anger certainly helps process it. But SCM API update was only a problem for GitHub Branch Source and BitBucket Branch Source plugins.

I didn't even need to update Subversion as it was fine with the updated SCM API.

I only updated GIT and Mercurial because they could both significantly benefit from the event apis, (and because I needed to in order to fix GitHub Branch Source and BitBucket Branch Source plugins), but from a "oh we upgraded SCM API" perspective, the 1.x -> 2.x transition for SCM API does not mandate an upgrade to the Git plugin (but we gain improvements from the update)

SCM API is designed to be as minimal as possible.

In the longer term, we're probably going to pull all the SCM stuff out of core and into SCM API where we can provide plugin authors with better guidance and try to consolidate and tidy up a lot of the mess that has evolved in the various plugins (everyone implementing their own thread pools for polling; lack of consolidated eventing; etc)

I would really like to know what your objection is to SCM API

stephenc · 2017-02-07T19:25:41Z

For example credentials-plugin - nobody knows how to use it.

Next on my todo list are implementation, consumer and user guides for credentials plugin.

KostyaSha · 2017-02-07T19:26:41Z

stuff out of core

Like
https://issues.jenkins-ci.org/browse/JENKINS-31153?focusedCommentId=246004&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-246004

(everyone implementing their own thread pools for polling; lack of consolidated eventing; etc)

GH plugin implementing eventing and it works fine it doesn't require to hack multiple plugins. I have good experience with github-plugin API + github-integration/pullrequest-plugins. They work great without any additional plugins, dependency tree is/was clear.
Common APIs doesn't work well, they always limited and locked with long reviews/releases (fix for workflow trigger mess for example).

Keeping in mind that i want switch one day from github-api library i'm not so strong on polluting this plugin ;)

KostyaSha · 2017-02-07T19:28:27Z

src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java

+    /**
+     * Called when a POST is made.
+     */
+    public void onPost(final String origin, String triggeredByUser) {


S maybe replace it with more common object with fields that will be expandable in future?

stephenc · 2017-02-07T19:30:39Z

So just to show useless information in UI you are adding useless hard dependencies, deprecating and hacking everything around?

We have customers with phantom builds that they don't know where they are being triggered from. I have seen non-customer users with the same problem.

So I disagree that the information is useless. It is only useless until you have builds being triggered causing unnecessary deployment int production... when you have that situation it is essential.

I am bumping a dependency that this plugin should really be depending on. Triggering scm builds will need to be consolidated into the scm event path for this plugin anyway.

KostyaSha · 2017-02-07T19:43:25Z

Origin is only one of multiple things that may be needed for debugging. System logger is standart pattern for debugging issues in jenkins. (everyone implementing their own thread pools for polling; lack of consolidated eventing; etc) exactly this appeared because nobody do design before implementation. Hack, merge, release, keep 10y ... make support business... Single string looks the same. If you would need one more you will create GitHubTrigger3, GitHubTrigger4??

KostyaSha · 2017-02-07T19:46:22Z

src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java

@@ -75,6 +77,13 @@ public void onPost() {
     * Called when a POST is made.
     */
    public void onPost(String triggeredByUser) {
+        onPost(SCMEvent.originOf(Stapler.getCurrentRequest()), triggeredByUser);


So why not call Stapler.getCurrentRequest() in GitHubTrigger implementation, it still the same thread?

KostyaSha

From all what i see (threading) you can call Stapler.getCurrentRequest() in eventsubscriber/trigger impl

stephenc · 2017-02-07T19:55:19Z

From all what i see (threading) you can call Stapler.getCurrentRequest() in eventsubscriber/trigger impl

That assumes that all callers of the api are doing the call from a stapler request.

If somebody is relaying requests between masters, there may be no request but the origin has been relayed (and relay appended)

Hence better to have the origin as an explicit parameter

stephenc · 2017-02-07T20:01:39Z

System logger is standart pattern for debugging issues in jenkins.

And that is really crappy when trying to trace why a job got built. The logs have rotated away before you can find out.

Jobs already retain the cause of their build. They should include tracing information for that cause.

KostyaSha · 2017-02-07T20:02:02Z

If somebody is relaying requests between masters, there may be no request but the origin has been relayed (and relay appended)

So it only CB enterprise issue?

Hence better to have the origin as an explicit parameter

So wrap it to object along with triggeredByUser it will be possible extend it in future.

KostyaSha · 2017-02-07T20:05:46Z

System logger is standart pattern for debugging issues in jenkins.
And that is really crappy when trying to trace why a job got built. The logs have rotated away before you can find out.

Jobs already retain the cause of their build. They should include tracing information for that cause.

Crap is system settings that requires restart master, you and other still adding them into jenkins and plugins and ignoring my suggestions to stop doing it without ui/system settings.

https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitSCM.java#L1782 wonderful example! Introduced because some people don't want to see verbose/too much information about build.

KostyaSha · 2017-02-08T01:39:57Z

processEvent() and everything else still on String arg

jglick · 2017-02-08T14:32:05Z

src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java

+        }
+
+        @Override
+        public String toString() {


Gratuitous for a throwaway builder.

perhaps... until you have to debug one

I usually placing reflection tostring builder because people always forget to add new fields. And in any way jenkins can't work with java security.

lanwen

Personally like idea with wrapper-objects for events.

But GithubTrigger2 smells like very bad thing. I'd like to avoid. Its already ignoring interface to search trigger: triggerFrom(job, GitHubPushTrigger.class) - so we don't call interface anywhere - isn't it?

lanwen · 2017-02-08T15:31:29Z

src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java

+    }
+
+    @Override
+    public boolean equals(Object o) {


time to try lombok?

IMO way more magic danger than it is worth.

ok, agree, should be tested separately

lanwen · 2017-02-08T15:39:03Z

src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java

@@ -57,7 +59,7 @@
 *
 * @author Kohsuke Kawaguchi
 */
-public class GitHubPushTrigger extends Trigger<Job<?, ?>> implements GitHubTrigger {
+public class GitHubPushTrigger extends Trigger<Job<?, ?>> implements GitHubTrigger2 {


can we just drop this Interface implementation? Do you know somebody who use it?

You can't know who is using it.

so we can just continue to implement old interface (marked as deprecated) and do nothing with its methods and just use brand new method

Or switch to java8?

lanwen · 2017-02-08T15:43:39Z

...est/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventListenerTest.java


-        verify(trigger).onPost(TRIGGERED_BY_USER_FROM_RESOURCE);
+        verify(trigger).onPost(GitHubTriggerEvent.create()


is it should be verify(trigger).onPost(same(...)) (with matcher?)

KostyaSha · 2017-02-09T17:50:27Z

src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java

@@ -87,6 +102,9 @@ private boolean runPolling() {
                        PrintStream logger = listener.getLogger();
                        long start = System.currentTimeMillis();
                        logger.println("Started on " + DateFormat.getDateTimeInstance().format(new Date()));
+                        if (event.getOrigin() != null) {
+                            logger.format("Started by event from %s on %tc%n", event.getOrigin(), event.getTimestamp());


if you made toString, then simply print event

KostyaSha · 2017-02-09T17:52:36Z

src/main/java/org/jenkinsci/plugins/github/extension/GHSubscriberEvent.java

+ *
+ * @since 1.25.2
+ */
+public class GHSubscriberEvent extends SCMEvent<String> {


So you are adding hard dependency on plugin that community never discussed to design?

KostyaSha · 2017-02-09T18:02:14Z

we're probably going to pull all the SCM stuff out of core and into SCM API

Where it and it's design was discussed with community/devs? https://github.com/jenkinsci/scm-api-plugin/graphs/contributors How that happened that your plugin become hard dependency for everybody and everybody should depend on it?

KostyaSha · 2017-02-09T18:23:55Z

Opening release sources https://github.com/jenkinsci/scm-api-plugin/tree/d03e1d08c8cc800e3edc361b3b313bf69ece67ef no docs, how at all it happenned to be dependency for everybody...

stephenc · 2017-02-09T17:04:59Z

src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java

+        }
+
+        @Override
+        public String toString() {


perhaps... until you have to debug one

stephenc · 2017-02-09T17:17:28Z

src/main/java/com/cloudbees/jenkins/GitHubTrigger.java

@@ -15,6 +15,7 @@
 * and triggers a build.
 *
 * @author aaronwalker
+ * @deprecated extend {@link GitHubTrigger2} instead


I wonder if this interface is required for https://github.com/jenkinsci/github-sqs-plugin/blob/master/src/main/java/com/base2services/jenkins/SqsBuildTrigger.java or if it is a gratuitous linkage between the two plugins. It is the only OSS consumer of this interface... the only reason for it to exist is so that the two trigger options will both work for triggering builds... which seems a really crappy UX

i.e. is it obvious to the user that checking one or both of the above check boxes will do exactly the same thing?

And those check boxes are kind of against the idiomatic Jenkins way for configuring hooks, i.e. through Poll SCM

I really think all this needs a good tidy up... but I am not going to take that on in this PR ;-)

stephenc · 2017-02-09T17:19:33Z

src/main/java/com/cloudbees/jenkins/GitHubTrigger.java

@@ -15,6 +15,7 @@
 * and triggers a build.
 *
 * @author aaronwalker
+ * @deprecated extend {@link GitHubTrigger2} instead


(FTR the "Build when a message is published to an SQS Queue" sounds more that a bit scary... what kind of message and what queue? Does it mean that every time any message is pushed to any SQS Queue anywhere in the world that my build will be triggered?)

stephenc · 2017-02-09T17:26:53Z

src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java

@@ -85,14 +86,19 @@ protected void onEvent(GHEvent event, String payload) {
                @Override
                public void run() {
                    for (Item job : Jenkins.getInstance().getAllItems(Item.class)) {
-                        GitHubTrigger trigger = triggerFrom(job, GitHubPushTrigger.class);
+                        GitHubPushTrigger trigger = triggerFrom(job, GitHubPushTrigger.class);


Highlights the pointlessness of the GitHubTrigger interface when the only other user could avoid it completely: https://github.com/jenkinsci/github-sqs-plugin/blob/d5886f8c3669e4ca61c1c359cd216aca713c7aeb/src/main/java/com/base2services/jenkins/github/GitHubTriggerProcessor.java#L87 which doesn't need to down-cast to the interface type as it knows that the triggerClass is always SqsBuildTrigger

stephenc · 2017-02-09T21:30:50Z

@KostyaSha

Where it and it's design was discussed with community/devs?

Well the current long term goals is to move most stuff out of core and into separate plugins.

While we could pull the existing (crappy) SCM classes into their own plugin, to my mind it makes more sense to move them into SCM API.

When/if it comes time to move them, rest assured that - if I am involved - there will be a discussion with the community.

https://github.com/jenkinsci/scm-api-plugin/graphs/contributors How that happened that your plugin become hard dependency for everybody and everybody should depend on it?

Well most people see the multi-branch stuff as the way forward, SCM API is the API that these things are built on top of. I have tried to keep separation of concerns (with the added cognitive costs involved) such that the multibranch stuff is all in Branch API while the pure SCM related stuff is in SCM API.

I have provided guides for people to develop competing alternatives to Branch API. Just because I see Branch API as the best approach does not mean I am arrogant and believe it is the "one true solution".

Ideally the SCMSources should actually be part of the core SCM plugins, so for example Subversion and Git plugins have SCMSources built in. TBH this plugin should really be the plugin with the SCMSource for GitHub, but that is not the case and so we have a separate plugin just to add the SCMSource implementations - which complicates things for users.

How many people are complaining about the efforts I am putting into the SCM API and Branch APIs? From what I can see (other than bugs that need fixing) almost everyone likes the SCM API and Branch API approach - but perhaps people are afraid to speak up (which saddens me).

I really do not want to go down the hard-fork path that you seem to want to push everyone towards. I hope this is just another case of "Lost in Translation"

stephenc · 2017-02-09T22:07:03Z

I think the test failure is unrelated:

Exception: java.lang.OutOfMemoryError thrown from the UncaughtExceptionHandler in thread "Executing shouldTurnOnOverridingWhenThereIsCredentials(com.cloudbees.jenkins.GlobalConfigSubmitTest)"

KostyaSha · 2017-02-09T22:07:29Z

How many people are complaining about the efforts I am putting into the SCM API and Branch APIs? From what I can see (other than bugs that need fixing) almost everyone likes the SCM API and Branch API approach - but perhaps people are afraid to speak up (which saddens me).

I really do not want to go down the hard-fork path that you seem to want to push everyone towards. I hope this is just another case of "Lost in Translation"

No, you didn't get my point. Direction is good, but it done in the same way as started contributing in jenkins. I can show example, you blocked CloudProvisioningRequest enhancement PR and most of improvements for cloud in core, then you say that you want cloud-api-plugin, ok. People ready to participate (i.e. me), but nothing can't happen without devs from company X. I definitely sure that acegi will be replaced with spring-security only when it will be needed for company X needs.

I'm also J developer (unfortunately) and reading development mail list. I don't remember any related discussion, i see only that somebody came from company X and says "it's future, use it". Docs? Later! Just merge and use plugin XX. Where is communication with community (if it still exists)?
I wrote new api for GH triggering and was ready to merge this stuff into github-plugin to make great plugin (but was forced to run away from jenkinsci because jenkins and jenkins infra are locked on single vendor), was ready to make multibranch, but... ? No docs, wait for docs. Ok! Now there is github-multibranch that makes 2 gh trigger useless, and full of bad practises of 1 old plugin. There is no any guarantee that you wouldn't decide to merge branch-source into scm/branch plugins or make something that will suddenly deprecate or stop working from upstream dependency.
When scm-api appeared in dependency it had only few simple classes and now it monster (that reminds me one more plugin 😃 ) .

almost everyone likes the SCM API and Branch API approach

Most of people even doesn't know that such plugins exists. Like != code and use it. Documentation appeared only in december afair. I got description from @jglick not so long ago. I don't talk even about dependency mess. One of solution that i saw from people - remove workflow plugins from production instance. But it other story.

really do not want to go down the hard-fork path that you seem to want to push everyone towards.

I push to do better communication/design BEFORE doing something and not AFTER ("deal with it"). Mine rewritten (in some sense forked by functionality) plugins doesn't affect any other plugins, so i have no need to disturb anybody (excluding libraries like github-api). org-folder, branch/scm/etc affects everybody, but not discussed with everybody. You can get release/fix for any plugin you want, community - can't.

KostyaSha · 2017-02-09T22:08:52Z

I think the test failure is unrelated:

That's java 😄 If it pass locally, then ok.

stephenc · 2017-02-09T22:13:36Z

No, you didn't get my point. Direction is good, but it done in the same way as started contributing in jenkins. I can show example, you blocked CloudProvisioningRequest enhancement PR and most of improvements for cloud in core, then you say that you want cloud-api-plugin, ok. People ready to participate (i.e. me), but nothing can't happen without devs from company X.

Rest assured I am very frustrated that I have not been given time to fix the Cloud API problems

I definitely sure that acegi will be replaced with spring-security only when it will be needed for company X needs.

Actually I have a plan to bypass the need to do that... and I have agreement to let me do some quick experiments, but right now I am trying to tidy up some other bigger messes

KostyaSha · 2017-02-09T22:17:03Z

Actually I have a plan to bypass the need to do that... and I have agreement to let me do some quick experiments, but right now I am trying to tidy up some other bigger messes

And then it ends with security-plugin? 🤣
Replace SecurityFilters.groovy initialisation in Realm with spring-security classes chains + write hudson security classes based on spring-security? Right now frustrating and f... with security chains (hope i didn't end with fork).

stephenc · 2017-02-09T22:17:29Z

Most of people even doesn't know that such plugins exists. Like != code and use it. Documentation appeared only in december afair.

Yes, I agree that the documentation should have been written sooner, but there is documentation now and I even documented a clear clean path for implementing the old-style SCM API in a modern way.

I got description from @jglick not so long ago.

I don't talk even about dependency mess. One of solution that i saw from people - remove workflow plugins from production instance. But it other story.

What is the dependency mess in SCM API? SCM API specifically has no additional runtime dependencies

Now if you are talking about the maze of pipeline dependencies, I hear you, but how is that relevant to this plugin?

KostyaSha · 2017-02-09T22:20:32Z

Now if you are talking about the maze of pipeline dependencies, I hear you, but how is that relevant to this plugin?

I don't remember details but some scm related stuff was moved from workflow plugins into git and then everything bomb. Maybe not related directly to scm, but related to use/do modern apis.

stephenc · 2017-02-10T09:09:23Z

@lanwen are we good to go?

lanwen · 2017-02-10T14:01:46Z

released as 1.26.0

KostyaSha · 2017-02-10T16:06:28Z

@lanwen please squash/fixup commits in one without mentioning user next time. Now i have useless highlights from commits.

[FIXED JENKINS-41811] Expose event origin to listeners

ac334ce

stephenc commented Feb 7, 2017

View reviewed changes

KostyaSha reviewed Feb 7, 2017

View reviewed changes

KostyaSha requested changes Feb 7, 2017

View reviewed changes

KostyaSha reviewed Feb 7, 2017

View reviewed changes

jglick suggested changes Feb 7, 2017

View reviewed changes

stephenc added 2 commits February 7, 2017 17:11

[JENKINS-41811] Address code review comments

7adda3b

[JENKINS-41811] SCM API 2.0.3

366b786

stephenc changed the title ~~[WIP] [FIXED JENKINS-41811] Expose event origin to listeners~~ [FIXED JENKINS-41811] Expose event origin to listeners Feb 7, 2017

jglick approved these changes Feb 7, 2017

View reviewed changes

Remove unused imports

3515775

KostyaSha reviewed Feb 7, 2017

View reviewed changes

KostyaSha requested changes Feb 7, 2017

View reviewed changes

stephenc force-pushed the jenkins-41811 branch from fed2636 to 45ade81 Compare February 8, 2017 00:25

[JENKINS-41811] Move more to event objects

e2db2f8

stephenc closed this Feb 8, 2017

stephenc reopened this Feb 8, 2017

jglick approved these changes Feb 8, 2017

View reviewed changes

lanwen requested changes Feb 8, 2017

View reviewed changes

[JENKINS-41811] We don't need no GitHubTrigger2 interface

a0567d6

stephenc force-pushed the jenkins-41811 branch from 76cf356 to a0567d6 Compare February 9, 2017 17:04

[JENKINS-41811] Fix up tests

622467e

KostyaSha reviewed Feb 9, 2017

View reviewed changes

stephenc commented Feb 9, 2017

View reviewed changes

[JENKINS-41811] Fix up code-quality build

148bf7f

KostyaSha approved these changes Feb 9, 2017

View reviewed changes

lanwen approved these changes Feb 10, 2017

View reviewed changes

lanwen merged commit 78167fd into jenkinsci:master Feb 10, 2017


		verify(trigger).onPost(TRIGGERED_BY_USER_FROM_RESOURCE);
		verify(trigger).onPost(GitHubTriggerEvent.create()

[FIXED JENKINS-41811] Expose event origin to listeners #164

[FIXED JENKINS-41811] Expose event origin to listeners #164

Conversation

stephenc commented Feb 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KostyaSha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jglick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephenc commented Feb 7, 2017

KostyaSha commented Feb 7, 2017

stephenc commented Feb 7, 2017

KostyaSha commented Feb 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KostyaSha commented Feb 7, 2017

stephenc commented Feb 7, 2017

stephenc commented Feb 7, 2017

KostyaSha commented Feb 7, 2017

Choose a reason for hiding this comment

stephenc commented Feb 7, 2017

KostyaSha commented Feb 7, 2017 • edited Loading

Choose a reason for hiding this comment

KostyaSha left a comment

Choose a reason for hiding this comment

stephenc commented Feb 7, 2017

stephenc commented Feb 7, 2017

KostyaSha commented Feb 7, 2017

KostyaSha commented Feb 7, 2017

KostyaSha commented Feb 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KostyaSha Feb 9, 2017 • edited Loading

Choose a reason for hiding this comment

lanwen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lanwen Feb 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KostyaSha commented Feb 9, 2017 • edited Loading

KostyaSha commented Feb 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephenc commented Feb 9, 2017

stephenc commented Feb 9, 2017

KostyaSha commented Feb 9, 2017

KostyaSha commented Feb 9, 2017

stephenc commented Feb 9, 2017

stephenc commented Feb 7, 2017 •

edited

Loading

KostyaSha commented Feb 7, 2017 •

edited

Loading

KostyaSha Feb 9, 2017 •

edited

Loading

lanwen Feb 8, 2017 •

edited

Loading

KostyaSha commented Feb 9, 2017 •

edited

Loading