Details
- Reviewers
pulkit - Group Reviewers
hg-reviewers - Commits
- rHG5997eabc7b85: push: added clear warning message when pushing closed branches(issue6080)
Diff Detail
- Repository
- rHG Mercurial
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Event Timeline
tests/test-push-warn.t | ||
---|---|---|
235 | Minor nit, but it looks like the existing space before the branch name was lost. And it would probably be a little more readable if there was a space between the branch name and (closed). |
mercurial/discovery.py | ||
---|---|---|
358 | I had assumed from the bug report that the request was to annotate each closed branch as closed, so I interpreted the output incorrectly. It might be too much output to put (closed) after each closed branch in the list if it is long. But maybe "(%d closed)" % len(closedbranches) will make it less ambiguous? See what others think. |
mercurial/discovery.py | ||
---|---|---|
358 | maybe we can just add (contains closed branches) at the end of the message. |
mercurial/discovery.py | ||
---|---|---|
358 | okay i will update the patch . |
Nice work, I have left some inline comments. Can you also add a test where we pushing multiple branches and not every branch is a closed branch?
mercurial/discovery.py | ||
---|---|---|
348 | How about having this as a set so that we don't need to convert it later. | |
350 | if isclosed will work. | |
352 | No need to convert closedbranches back to list. | |
356–362 | if closedbranches will work here. |
@taapas1128 when you get time, can you see whether reading the branchmap can slow down things or not. It other words, can you test some performance numbers with it?
A good way to do that is to enable contrib/perf.py as perf extension and use hg perfdiscovery command.
How about having this as a set so that we don't need to convert it later.