Closed melvyndekort closed 10 years ago
:+1:
Works on my setup.
Just wanted to let you know that I have merged this locally and am testing it now. I've done a little bit of refactoring to the EligibilityFilter chain setup so two methods aren't required to be implemented for each item in the filter. Going through some final testing and hope to have it ready to go by the weekend.
Great, thank you for your heads-up!
Groeten,
Melvyn de Kort
On Thu, Dec 12, 2013 at 1:12 PM, Michael Irwin notifications@github.comwrote:
Just wanted to let you know that I have merged this locally and am testing it now. I've done a little bit of refactoring to the EligibilityFilter chain setup so two methods aren't required to be implemented for each item in the filter. Going through some final testing and hope to have it ready to go by the weekend.
— Reply to this email directly or view it on GitHubhttps://github.com/Nerdwin15/stash-jenkins-postreceive-webhook/pull/39#issuecomment-30414840 .
Anymore update @mikesir87 I'm waiting on the new jar build to update mine as my Jenkins seems to be messing up.
Michael,
I realize that we just had the holiday season and that you have probably spent your time with family and loved ones. But I am wondering if there might be something wrong with my pull request that has kept you from merging it and building a new release. Can you give an update on the status?
No problem at all. I did get busy with the holiday and tried to minimize my amount of time on the computer. Thanks for your patience with this too. :)
I still haven't been able to get my Jenkins instance to actually get the pull request refs. But, since you have it working in your instance, I'm planning to push up the new code (which includes this merge), then letting you test it out. If it works from there, I'll go ahead and push it to the Atlassian Marketplace for everyone else. It's not worth holding everyone else up for me to try and get it working when others already have it working.
Sound alright?
Sounds good, but if you want to I can help you setup your test environment. However I think this is an issue we should discuss in a more direct setting, like Skype or something.
Agreed. I'll get in touch with you here soon.
I've just been testing your latest commit (2.1.0-SNAPSHOT), and I found an issue. It might have been already in my commit, but I haven't checked.
When a new pull-request is created, it doesn't trigger a build. However, when a new commit is pushed to an existing pull-request, it does trigger a build. This means that just creating a pull-request is not enough to trigger a build, a second commit must be pushed too. Of course this is an unwanted situation, I'll start working on a solution.
It turns out we also need to check for the PullRequestOpenedEvent next to the PullRequestRescopedEvent. I'm looking into it now, I think I've focused too much on the updates of pull-requests and forgotten about the initial pull-requests. This is probably also the reason why you couldn't test the functionality on your setup. I think if you would push a commit to an existing pull-request, it would trigger a build.
Ah. That might be what it is then. Should be a pretty quick fix. I'll get it updated in just a minute.
I think I've already fixed it, I'm working on the tests now, unless you want to create those yourself?
Doesn't matter to me. Go ahead and do it and submit a pull request. May want to add in the PullRequestReopenedEvent while you're at it too.
Already added the PullRequestReopenedEvent, making the pull request now ;-)
Awesome. (and man... you're quick at replies ;-) )
I just like the e-mail notification of github ;-)
Hi Michael,
I've done some hacking and I think i might have solved the pull request issue. Please take a look at my code and use whatever you like.
Kind regards,
Melvyn de Kort