#2445 Pagure hook does not work for non-master branches
Opened 6 years ago by mikem. Modified a year ago

I manually merged a Koji PR that targeted the legacy-py24 branch


The commit message contained the line Merges #520, yet when I pushed the changes, the Pagure hook did not close the pr as it normally does.

Granted, perhaps this is intentional. I expect there are folks using non-master public branches for development (e.g. a devel or testing branch), and it might be confusing to close a pr or issue when it has only been pushed to devel or testing. OTOH, if the PR in question is targeting that branch, then perhaps it would make sense regardless.

I'm not sure what the best answer is here, but I think at least the behavior for non-master branches should be clarified in the docs.

You are correct https://pagure.io/pagure/blob/master/f/pagure/hooks/files/pagure_hook.py#_184-187 we are skipping what is not the main branch.

+1 on fixing the doc on this.

Metadata Update from @pingou:
- Issue tagged with: bug, doc, easyfix

6 years ago

I wonder if this makes sense:

  • scan for merges/fixes
  • if branch == master: do the closes/merges
  • if branch != master:
    • if no merged pr#, do nothing
    • if (all) merged prs target this branch:
    • do the closes merges
  • if linked prs/issues were not merged/closed, then output the reason, perhaps with a link to docs

If a PR has a branch target other than master, and I manually merge it to that branch with the appropriate "Merges: #NNN" line, then I think really think Pagure ought to at least close the PR. I can see an argument against closing linked issues (since they have no branch affiliation), but Pagure knows whether or not the branch being updated matches the target branch of the PR.

Conversely, if a PR targets a branch that isn't master, Pagure should probably not close that PR if it is manually pushed to master.

Metadata Update from @pingou:
- Issue assigned to pingou

6 years ago

Running across this again as I work on PRs for a Koji update release. Any change of seeing changes here soon? Does my suggested behavior make sense?

Hello @pingou, I am new here and would like to work on this issue and make my 1st contribution.

@deveshverma cool, let us know if we can help you in any way :)

as far I can see no contribution or further progress, worth to double check, adding it to the 5.13 milestones.

Metadata Update from @wombelix:
- Issue set to the milestone: 5.13

a year ago

Login to comment on this ticket.