.\s is equivalent to .* which doesn't seem to be intended
(?:.*\s+)? will match the empty string, or arbitrary space-separated prefixes
Existing code will match some things that were probably not intended. E.g.
p = re.compile(r'.\smerge?[sd]?:?\s*?#(\d+)', re.I) p.match('nowhitespacemerge: #45').groups() ('45',) p.match('This commit unmerges #45').groups() ('45',)
Just to be sure, did you run the test suite?
I'm also thinking if it might be worth adding the invalid case you suggested to make this doesn't fail in the future
I could not run it actually. Doesn't like f22. I did test the regexes. If I can sort that out, I'd be happy to add resubmit with some test cases
I could not run it actually. Doesn't like f22.
Could you maybe catch me on IRC with the traceback?
Otherwise the tests for the regex are in: https://pagure.io/pagure/blob/master/f/tests/test_pagure_lib_link.py
It was failing from missing deps. I manually rebuild a few fc23 packages and got it working.
So... yeah, turns out my change does break the test, but only because the tests depend on the index of the regex in the list (which seems wrong).
I'll update this PR with some updated tests later. Don't have much time to look at it today.
2 new commits added
This passes the updated tests. Let me know what you think. I carried the existing test strings over and added some new ones.
I am now realizing that this is a bug, we should retrieve ('pingou', 'test', 1) otherwise we will be closing a ticket on the original project instead of the fork, looks like the bug was here before but your changes are making it apparent.
('pingou', 'test', 1)
The changes are looking really nice, they also showed that there was a bug in my regex.
Would you like to see about fixing them or should we do that in another PR (if so I'll merge this one)?
It looks like more than just the regex. When get_relation() searches for related issues, it seems to only consider a single repo:
repo = pagure.lib.get_project(session, reponame, user=username) .... relation = pagure.lib.search_issues(session, repo=repo, issueid=relid)
I guess in the forks case, we'd need to call get_project() again for the other repo and adjust the search for that case. What would I need to pass to get_project() to get the correct forked repo? reponame="$user/$project"?
What would I need to pass to get_project() to get the correct forked repo?
It would need a user=username to get the fork: repo = pagure.lib.get_project(session, repo, user=username) if username is None, then it return the main project, otherwise it searches for a fork.
user=username
repo = pagure.lib.get_project(session, repo, user=username)
None
1 new commit added
Looked at it a little. Maybe best to pull this in and do another PR to fix handling of fork urls.
Also, updated the text in in the hook description.
Ok, would you mind to rebase your branch on the top of master then?
Thanks
rebased
Pull-Request has been merged by pingou
.\s is equivalent to .* which doesn't seem to be intended
(?:.*\s+)? will match the empty string, or arbitrary space-separated prefixes