( )⚙ D9316 log: add argument -B/--bookmark for bookmarks

This is an archive of the discontinued Mercurial Phabricator instance.

log: add argument -B/--bookmark for bookmarks
AbandonedPublic

Authored by pulkit on Nov 12 2020, 8:11 PM.

Details

Reviewers
martinvonz
marmoute
sebhtml
Group Reviewers
hg-reviewers
Summary

This is the v2 of my patch series for adding bookmark support to "hg log".

v2

Previous version:
v1

Mercurial upstream version for v2:
hg-committed ede4a1bf14bd (@ bookmark)

Operating system:
FreeBSD atlantis 12.2-RELEASE FreeBSD 12.2-RELEASE r366954 GENERIC amd64

CPU: AMD Ryzen 5 2600 Six-Core Processor

RAM: 8 GB ECC

Python: Python 3.7.6

Shell:
GNU bash, version 5.0.18(3)-release (amd64-portbld-freebsd12.1)

Test Plan

Test command:
python3.7 ./tests/run-tests.py

Failed tests:

Ran 871 tests, 71 skipped, 20 failed.

Failed test-bookmarks-pushpull.t#b2-binary: timed out
Failed test-bookmarks-pushpull.t#b2-pushkey: timed out
Failed test-duplicateoptions.py: output changed ("failed to import extension git: No module named 'git'")
Failed test-glog-beautifygraph.t: timed out
Failed test-glog.t: timed out
Failed test-import.t: timed out
Failed test-largefiles-misc.t: timed out
Failed test-largefiles.t: timed out
Failed test-lfs-serve.t#lfsremote-on: output changed ("error: conflicting output format options.")
Failed test-log.t: timed out
Failed test-merge-combination.t: timed out
Failed test-mq.t: timed out
Failed test-obsmarker-template.t: timed out
Failed test-obsolete-bundle-strip.t: timed out
Failed test-obsolete.t: timed out
Failed test-persistent-nodemap.t: timed out
Failed test-rebase-obsolete.t: timed out
Failed test-revset.t: timed out
Failed test-subrepo.t: timed out
Failed test-template-functions.t: timed out

I don't know the why some of the tests are doing a time out.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

sebhtml created this revision.Nov 12 2020, 8:11 PM
sebhtml edited the summary of this revision. (Show Details)Nov 12 2020, 8:15 PM
sebhtml edited the test plan for this revision. (Show Details)
sebhtml added reviewers: martinvonz, pulkit.
sebhtml edited the summary of this revision. (Show Details)Nov 12 2020, 8:17 PM
sebhtml edited the test plan for this revision. (Show Details)

IMO, these three commits should be folded/squashed into a single commit. One way to think of it is this: if we decide to roll it back, would it be okay to roll back only one of them?

Also, most of the commit message is noise to me. You can safely remove all of it, IMO. I would instead like the commit message to tell me what the feature does and why it's useful. That includes explaining that it shows the commits that are reachable only by the specified bookmark. Also explain that that matches the semantics of hg export (and whatever other commands it was that I pointed you to last time). Thanks.

I don't know the why some of the tests are doing a time out.

Some of them are just really slow. I usually pass -t 600 to give them more time to finish.

martinvonz requested changes to this revision.Nov 16 2020, 1:08 PM
This revision now requires changes to proceed.Nov 16 2020, 1:08 PM

IMO, these three commits should be folded/squashed into a single commit. One way to think of it is this: if we decide to roll it back, would it be okay to roll back only one of them?
Also, most of the commit message is noise to me. You can safely remove all of it, IMO. I would instead like the commit message to tell me what the feature does and why it's useful. That includes explaining that it shows the commits that are reachable only by the specified bookmark. Also explain that that matches the semantics of hg export (and whatever other commands it was that I pointed you to last time). Thanks.

I don't know the why some of the tests are doing a time out.

Some of them are just really slow. I usually pass -t 600 to give them more time to finish.

Got it.

So... have a question, regarding the process to follow.

Since you recommend that I put my 3 patches in 1 commit, and that I rewrite the commit message as you guided, would it make sense for me to

IMO, these three commits should be folded/squashed into a single commit. One way to think of it is this: if we decide to roll it back, would it be okay to roll back only one of them?
Also, most of the commit message is noise to me. You can safely remove all of it, IMO. I would instead like the commit message to tell me what the feature does and why it's useful. That includes explaining that it shows the commits that are reachable only by the specified bookmark. Also explain that that matches the semantics of hg export (and whatever other commands it was that I pointed you to last time). Thanks.

I don't know the why some of the tests are doing a time out.

Some of them are just really slow. I usually pass -t 600 to give them more time to finish.

Hello,

I addressed your comments, see my V3 here https://phab.mercurial-scm.org/D9341

pulkit edited reviewers, added: sebhtml; removed: pulkit.Dec 4 2020, 2:30 PM
pulkit commandeered this revision.

Okay seems functionality implemented by this and later patches got pushed as D9341. Moving this out of review.

pulkit abandoned this revision.Dec 4 2020, 2:30 PM