| Summary: | Eclipse IP Validation not running on github pull requests | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Community | Reporter: | Dan Heidinga <danielheidinga> | ||||
| Component: | GitHub | Assignee: | Eclipse Webmaster <webmaster> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | charlie.gracie, charlie_gracie, danielheidinga, denis.roy, icraggs, joakim.erdfelt, mikael.barbero, nikhilnanivadekar, webmaster | ||||
| Version: | unspecified | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Mac OS X | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Dan Heidinga
Hmm I wonder if the right component for this is actually Github instead of CI-Jenkins? The hook is properly set and the events are properly sent to the hook. I guess the issue is on the hook side then. Denis, Matt, Could you please have a look? *** Bug 531696 has been marked as a duplicate of this bug. *** See the attachment on Bug 531696 shows the details of the actual github request and eclipse webhook response (headers and body contents). The response body contents seems like the webhook is initializing for the first time and is dumping php debug information on the response body content. Past webhook responses are much simpler and that's what's likely confusing github. *** Bug 531773 has been marked as a duplicate of this bug. *** So there does seem to be a couple of issues. First we're not able to connect back to Github(looks like an SSL issue). Second there is something goofy about the returned results when it does happen. I'll keep trying to work out what's happened. -M. What is the guidance to project maintainers regarding merging requests from contributors? Should we hold-off or we should merge? At this stage you'll need to confirm the sign-off and in the case of non-committers you should take a quick look at https://accounts.eclipse.org and use the 'user search' feature to confirm they have an ECA on file. I'm pretty sure I've got a solution, but I want to test it before claiming this is fixed(going forward). -M. Ok I've tested my changes and they seem to be working as I expect. Can I get confirmation? Unfortunately I don't see a way to add these reviews to existing pull requests. -M. Confirming that recent fix seems to be working. https://github.com/eclipse/jetty.project/pull/2281 Sweet! Please reopen if it happens again. -M. As a simple nit, clicking Details on the pull request sends me to this link: https://dev.eclipse.org/eclipse-webhook/services/status_details.php?id=5a9dd94484fe0 Which is a 403 IP Validation is not running again. For Example: https://github.com/eclipse/eclipse-collections/pull/479 (In reply to Nikhil Nanivadekar from comment #13) > IP Validation is not running again. > For Example: https://github.com/eclipse/eclipse-collections/pull/479 Hmmm, it's there now and seems to be happy. -M. Seems like it works fine on an existing PR only when the contributor rebases. If it is only a push, then IP Validation does not run. *** Bug 532542 has been marked as a duplicate of this bug. *** (In reply to Nikhil Nanivadekar from comment #15) > Seems like it works fine on an existing PR only when the contributor > rebases. If it is only a push, then IP Validation does not run. I think that's actually expected. Only project committers should be able to push, and they presumably know the IP rules. PRs may come from anywhere and so are checked. Wayne, have I got that wrong? -M. If a contributor pushes their own branch, and the PR is still open. The PR gets updated with the new commit. IP Validation should now run even on this new commit. This is how it was before. Created attachment 273727 [details]
Screenshot of IP Validation not working.
This seems to be broken again. Please refer to the screenshot attached. Reference PR: https://github.com/eclipse/eclipse-collections/pull/502 (In reply to Nikhil Nanivadekar from comment #20) > This seems to be broken again. Please refer to the screenshot attached. > Reference PR: https://github.com/eclipse/eclipse-collections/pull/502 Fixed. A small change hadn't been committed to master, so when an update was done it broke things. I've recovered that change and have pushed it to master. -M. |