Skip to content
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

Make notifications dependent on imported branch #1460

Closed
rockandska opened this issue Jan 17, 2019 · 3 comments
Closed

Make notifications dependent on imported branch #1460

rockandska opened this issue Jan 17, 2019 · 3 comments
Assignees
Milestone

Comments

@rockandska
Copy link

Feature Request

Use Case

Since travis-ci doesn't allow to trigger the notifications webhooks on specific branch, some roles are imported just because a user forked a repo to do a PR.
The original user could indeed configuring .travis.yml to only trigger the build on master, but it will disable the build on the branch created to made the PR on the forked repo too and will not allow the user who forked the project to check the travis build is OK before creating the PR without :

  • to be aware he need to changed .travis.yml with his own branch
  • rollback .travis.yml when his PR seems ok

Proposed Solution

Since the payload sent by travis-ci contain the branch who trigger the build, adding a parameter to the notifications api endpoint to only import the role if the branch built is contained in the parameter (on all branch if not set) could be maybe implemented ?

Implementation

Maybe something similar to this :

  • https://galaxy.ansible.com/api/v1/notifications/
    • import on all branch
  • https://galaxy.ansible.com/api/v1/notifications/?branch=master
    • only import if the branch who triggered the build is master
  • https://galaxy.ansible.com/api/v1/notifications/?branch=master:dev
    • only import if the branch who triggered the build is master or dev

Best regards,

@awcrosby
Copy link
Contributor

  1. When we get the travis notification webhook, currently regardless of the travis branch we import or re-import the default branch. We will make a change so: we will not import if the travis branch does not match the import branch (for existing repos) or the default branch (for new repos). So forking and pushing my_feature_branch should not trigger an import.
  2. When using the cli ansible-galaxy import and passing the --branch parameter, we will make sure that branch is what gets imported and also that branch is the only one that triggers the travis notification webhook
  3. We will remove the ansible-galaxy init meta/main.yml comment for the github_branch field since that field is not currently being used.
  • After this we will look into whether specifying the branch during the web UI import is something we want to add

@awcrosby awcrosby changed the title [FeatureRequest] Allow notifications on specific branch Make notifications dependent on imported branch Jan 22, 2019
@awcrosby awcrosby added the status/fix-committed Merged to develop \ release branch label Jan 29, 2019
@awcrosby
Copy link
Contributor

An additional change was made to address original feature request:

  1. If you give the webhook a branch query parameter, it will only trigger an import with pushes to that particular branch. For example a forked repo with a PR my_feature branch will not trigger an import with this: webhooks: https://galaxy.ansible.com/api/v1/notifications/?branch=master. Note that tag pushes will continue to trigger imports.

Associated PRs: #1475 and #1481

@rockandska
Copy link
Author

Thanks @awcrosby.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants