-
Notifications
You must be signed in to change notification settings - Fork 42
Allow release event #62
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
base: master
Are you sure you want to change the base?
Conversation
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.
ok
Is there any consideration to withhold this PR ? |
@aamsur-mkt I run into some problems with this PR, so need to check what is broken. |
if (!"push".equals(event) && !"release".equals(event)) { | ||
result.setStatus(403, "Only push or release events are accepted."); |
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.
Can the create
event be added as well ?
e.g.
if (!"push".equals(event) && !"release".equals(event) && !"create".equals(event)) {
result.setStatus(403, "Only push, release or create events are accepted.");
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.
Can the pull_request
event be added as well ?
e.g.
if (!"push".equals(event) && !"release".equals(event) && !"create".equals(event) && !"pull_request".equals(event)) {
result.setStatus(403, "Only push, release, create or pull_request events are accepted.");
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.
Same here as the above comment. Adding other events breaks things at the moment. Need to investigate how to fix this.
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.
@sanderv32 very interested in this work being completed and merged. Is there something I can contribute to help?
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.
The whole thing broke down with this changes. Haven't looked for a long time at this change, but if you want to test please do so.
Release events support was partially added in jenkinsci#62 but since its payload doesn't contain certain things, it was failing with return status being {"result":"ERROR","message":"net.sf.json.JSONException: JSONObject[\"commits\"] is not a JSONArray."}
Release events support was partially added in jenkinsci#62 but since its payload doesn't contain certain things, it was failing with return status being {"result":"ERROR","message":"net.sf.json.JSONException: JSONObject[\"commits\"] is not a JSONArray."}
Release events support was partially added in jenkinsci#62 but since its payload doesn't contain certain things, it was failing with return status being {"result":"ERROR","message":"net.sf.json.JSONException: JSONObject[\"commits\"] is not a JSONArray."}
Release events support was partially added in jenkinsci#62 but since its payload doesn't contain certain things, it was failing with return status being {"result":"ERROR","message":"net.sf.json.JSONException: JSONObject[\"commits\"] is not a JSONArray."}
This PR allow to also use the Gogs
release
event.