D6689: continue: added support for transplant

pulkit added inline comments. INLINE COMMENTS > transplant.py:737 > > +def continuetransplant(ui, repo): > +"""logic to resume an interrupted transplant using let's name this function `continue` only as it's in transplant.py and a continue implies that we are continuing transplant.

D6695: transplant: added support for --abort flag

pulkit added inline comments. INLINE COMMENTS > transplant.py:417 > +"""logic to abort an interrupted transplant""" > +if repo.vfs.exists('transplant/journal'): > +startctx = repo['.'] `canresume` above can be used for doing this check. REPOSITORY rHG Mercurial

D6697: cmdutil: add allowunfinished to prevent checkunfinished() on docommit()

This revision now requires changes to proceed. pulkit added a comment. pulkit requested changes to this revision. The test still fails. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6697/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6697

D6717: mypy: add a mypy.ini config file

pulkit added a comment. I am +1 on this. If I don't hear any objection on it until next week, I will queue it. Thanks for doing this work! REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6717/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6717

D6699: tests: add test for unshelve --interactive --keep

pulkit added a comment. You are already working on improving things with `--keep` and `--interactive`, so let's have this patch as a part of that patch. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6699/new/ REVISION DETAIL

D6659: graft: split graft code into seperate functions

pulkit added inline comments. INLINE COMMENTS > cmdutil.py:3431 > +"""logic to execute graft once revs are generated""" > +graftstate = statemod.cmdstate(repo, 'graftstate') > +for pos, ctx in enumerate(repo.set("%ld", revs)): let's initialize `graftstate` only where we need it. >

D6678: continue: added support for histedit

pulkit added inline comments. INLINE COMMENTS > histedit.py:1842 > > +def hgcontinuehistedit(ui, repo): > +state = histeditstate(repo) I see that we can have a unified function, `resumehistedit(..)` which can be called from `_histedit()` too. REPOSITORY rHG Mercurial CHANGES SINCE

D6689: continue: added support for transplant

pulkit added inline comments. INLINE COMMENTS > transplant.py:743 > +tp = transplanter(ui, repo, opts) > +tp.resume(repo, repo, opts) > +return I don't see `transplanter` modifying opts. So we can directly pass empty dict in both of the above function. >

D6696: abort: added support for transplant

pulkit added inline comments. INLINE COMMENTS > transplant.py:779 > +tp = transplanter(ui, repo, {}) > +tp.abort(ui, repo) > +return `return tp.abort(..)` instead. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6696/new/

D6695: transplant: added support for --abort flag

pulkit added inline comments. INLINE COMMENTS > transplant.py:416 > > +def clear(self): > +"""clear the state file if it exists""" We already have `self.unlog` doing the same here. > transplant.py:421 > + > +def abort(self, ui ,repo): > +"""logic to abort an

D6720: config: fix fm.data() handling of defaultvalue

pulkit added inline comments. INLINE COMMENTS > navaneeth.suresh wrote in commands.py:1884 > I did this but, couldn't see a difference in the test output. ah, I guess the output is sorted. Anyway, the suggested one is a correct way to do this, because otherwise we are writing name to fm two

D6712: config: fix defaultvalue template keyword (patch 1 of 2)

pulkit added a comment. This patch needs a better description. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6712/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6712 To: navaneeth.suresh, #hg-reviewers Cc: pulkit, durin42, marmoute,

D6720: config: fix defaultvalue template keyword (patch 2 of 2)

pulkit added a comment. This patch also needs a better description, something like: `config: fix fm.data() handling of defaultvalue`. INLINE COMMENTS > commands.py:1884 > fm.condwrite(ui.debugflag, 'source', '%s: ', source) > +fm.data(name=entryname,

D6712: config: fix defaultvalue template keyword (patch 1 of 2)

pulkit added a comment. The `pycompat.bytestr()` was there for py3 compatibility, however removing it should be fine in this case. Can you make sure `test-config.t` passes with this patch on Python 3? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION

D6709: config: add --registered flag to show all known configs

pulkit added inline comments. INLINE COMMENTS > ui.py:821 > > +def walkallconfig(self, untrusted=False): > +"""walk through all known config options in the registrar""" Since, we renamed the flag which was good idea, let's rename the function too. Also, this should better return

D6708: unshelve: clear shelvedstate and _finishunshelve() on partial unshelve

pulkit added a comment. Applied the following diff in flight which was result of test output changes. --- /home/pulkit/repo/paccess/tests/test-shelve.t +++ /home/pulkit/repo/paccess/tests/test-shelve.t @@ -1365,19 +1365,19 @@ #if stripbased $ hg log -r 3:: -G

D6694: unshelve: delete shelvedstate after a successful unshelve --continue

pulkit added inline comments. INLINE COMMENTS > test-shelve.t:1430 > +-- now, --continue should abort as the last unshelve was successful even > +-- though it was partially done. it can be completed without --continue. > + $ hg unshelve --continue the above sentence is confusing and maybe

D6708: unshelve: clear shelvedstate and _finishunshelve() on partial unshelve

pulkit added a comment. Sorry for not noticing this before, but this patch needs tests. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6708/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6708 To: navaneeth.suresh, #hg-reviewers, pulkit Cc:

D6694: unshelve: delete shelvedstate after a successful unshelve --continue

pulkit added a comment. This fails to apply on tip of default. Can you rebase and resend? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6694/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6694 To: navaneeth.suresh, #hg-reviewers, durin42,

D6694: unshelve: delete shelvedstate after a successful unshelve --continue

pulkit added inline comments. INLINE COMMENTS > test-shelve.t:1436 > + $ hg shelve --list > + default-01 (1s ago)changes to: add A to bars > + default (1s ago)changes to: add B to foo These timing needs to be globbed. REPOSITORY rHG Mercurial CHANGES SINCE LAST

D6686: unshelve: handle stripping changesets on interactive mode

pulkit added a comment. I am not sure why, but `test-shelve.t` fails for me with some hash changes. Can you have a look? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6686/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6686 To:

D6708: unshelve: clear shelvedstate and _finishunshelve() on partial unshelve

pulkit added inline comments. INLINE COMMENTS > shelve.py:1008 > if not ispartialunshelve: > _forgetunknownfiles(repo, shelvectx, addedbefore) > unshelvecleanup(ui, repo, basename, opts) IIUC, we don't need `_forgetunknownfiles()` too in this if statement. Can

D6699: tests: add test for unshelve --interactive --keep

pulkit added inline comments. INLINE COMMENTS > test-shelve.t:1476 > + > + $ hg shelve --list > + default (1s ago)changes to: add B to foo Can you show what the new shelve contains? Also add a test when there is partial unshelving going. REPOSITORY rHG Mercurial CHANGES

D6701: tests: sort imports in test-bookmarks-corner-case.t

Closed by commit rHGcfb774aae660: tests: sort imports in test-bookmarks-corner-case.t (authored by pulkit). This revision was automatically updated to reflect the committed changes. This revision was not accepted when it landed; it landed in state "Needs Review". REPOSITORY rHG Mercurial

D6702: py3: add one more test to list of passing tests

Closed by commit rHG6d50a7ebf7e0: py3: add one more test to list of passing tests (authored by pulkit). This revision was automatically updated to reflect the committed changes. This revision was not accepted when it landed; it landed in state "Needs Review". REPOSITORY rHG Mercurial CHANGES

D6702: py3: add one more test to list of passing tests

pulkit created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY Found by buildbot. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6702 AFFECTED FILES contrib/python3-whitelist CHANGE DETAILS

D6701: tests: sort imports in test-bookmarks-corner-case.t

pulkit created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY test-check-module-imports.t breaks on py3 without this change. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6701 AFFECTED FILES

D6697: cmdutil: add allowunfinished to prevent checkunfinished() on docommit()

pulkit added a comment. `test-commit-interactive.t` and some other tests fail for me with this, can you have a look? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6697/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6697 To:

D6697: cmdutil: add allowunfinished to prevent checkunfinished() on docommit()

This revision is now accepted and ready to land. pulkit added a comment. pulkit accepted this revision. > cmdutil: add allowunfinished to prevent checkunfinished() on docommit() s/docommit/dorecord/ in flight REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION

D6686: unshelve: handle stripping changesets on interactive mode

pulkit added a comment. In D6686#97957 , @navaneeth.suresh wrote: > I just found that this change is not required while creating D6694 . We will be needing the nodes which we are removing for later in

D6685: unshelve: changes how date is set on interactive mode

pulkit added a comment. > the remaining changes are shelved again for later. the shelve is updated with remaining changes. (mentioning update is important because we are not creating a new shelve, we are updating that shelve) > This patch modifies the date of remaining shelved

D6683: unshelve: unify logic around creating an unshelve changeset

pulkit added inline comments. INLINE COMMENTS > shelve.py:812 > +Here, we return both the newnode which is created interactively and a > +bool to know whether the shelve is partly done or completely done. > """ How about: Handles creation of unshelve commit and updating the

D6697: cmdutil: add allowunfinished to prevent checkunfinished() on docommit()

pulkit added inline comments. INLINE COMMENTS > cmdutil.py:274 > """ > -if not opts.get('interactive-unshelve'): > +if not (allowunfinished or opts.get('interactive-unshelve')): > checkunfinished(repo, commit=True) Why not get rid of `interactive-shelve`

D6684: unshelve: modify --continue on interactive unshelve

pulkit added a comment. Sorry for getting this and the parent patch split before, after the recent improvements, it looks like they should be folded into one. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6684/new/ REVISION DETAIL

D6688: unshelve: make basename as a mandatory argument for unshelvecontinue()

pulkit added a comment. In D6688#97899 , @navaneeth.suresh wrote: > In D6688#97859 , @pulkit wrote: > >> Can you explain in commit description as why this is done? > > This patch will become

D6685: unshelve: changes how date is set on interactive mode

pulkit added inline comments. INLINE COMMENTS > shelve.py:831 > user=shelvectx.user(), > -date=shelvectx.date()) > +date=dateutil.makedate()) > m = scmutil.matchfiles(repo, repo[snode].files()) We can just skip

D6684: unshelve: modify --continue on interactive unshelve

pulkit added inline comments. INLINE COMMENTS > shelve.py:950 > elif continuef: > -return unshelvecontinue(ui, repo, state, opts) > -elif len(shelved) > 1: Now I understand how this and the parent patches are arranged. Why don't be have all the logic related to

D6679: unshelve: store information about interactive mode in shelvedstate

pulkit added inline comments. INLINE COMMENTS > pulkit wrote in shelve.py:181 > We are already in shelve code, so no need to prepend shelved to the name. > > Also, we can only had `interactive` stored in the shelved state. If it's not > present in the state file, we can assume we were not

D6688: unshelve: make basename as a mandatory argument for unshelvecontinue()

pulkit added a comment. Can you explain in commit description as why this is done? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6688/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6688 To: navaneeth.suresh, #hg-reviewers Cc: pulkit,

D6686: unshelve: handle stripping changesets on interactive mode

pulkit added a comment. Can you add tests for this? There is no test which passes or fails because of this change. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6686/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6686 To: navaneeth.suresh,

D6685: unshelve: changes how date is set on interactive mode

pulkit added inline comments. INLINE COMMENTS > shelve.py:832 > user=shelvectx.user(), > -date=shelvectx.date()) > +date=opts.get('date')) > m = scmutil.matchfiles(repo, repo[snode].files()) IIUC `opts.get('date')`

D6684: unshelve: modify --continue on interactive unshelve

pulkit added inline comments. INLINE COMMENTS > test-shelve.t:1354 >C > - $ hg unshelve --continue -i <> y This test change should be in the parent differential. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6684/new/ REVISION DETAIL

D6676: unshelve: add help text on --interactive in verbose mode

pulkit added a comment. In D6676#97793 , @mharbison72 wrote: > Maybe I’m looking at something wrong, but it looks like a couple references to “the shelve” snuck in after the last edit in diff 16017. Ah, forgot to mention, I edited the

D6682: fsmonitor: add support for extra `hg debuginstall` data

This revision is now accepted and ready to land. pulkit added inline comments. pulkit accepted this revision. INLINE COMMENTS > __init__.py:182 > + ui.configpath("fsmonitor", "watchman_exe")) > +import tempfile > +root = tempfile.mkdtemp() Moved this import to top level in

D6681: debugcommands: add support for extensions adding their own debug info

This revision is now accepted and ready to land. pulkit added a comment. pulkit accepted this revision. Maybe worth to add about this in relnotes for 5.2 REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6681/new/ REVISION DETAIL

D6676: unshelve: add help text on --interactive in verbose mode

pulkit added a comment. Since, we already has `--interactive` pushed for the upcoming release, queuing this on stable branch. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6676/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6676 To:

D6679: unshelve: store information about interactive mode in shelvedstate

pulkit added inline comments. INLINE COMMENTS > shelve.py:181 > +_shelvedinteractively = 'shelvedinteractively' > +_notshelvedinteractively = 'notshelvedinteractively' > We are already in shelve code, so no need to prepend shelved to the name. Also, we can only had `interactive`

D6683: unshelve: fixes on interactive mode

pulkit added a comment. This patch does following things: 1. unify logic around creating unshelvectx 2. changes how date is set in iteractive mode 3. handles stripping in interavtive mode 4. compute a matcher only if it's required 5. and a change around making basename madatory

D6596: unshelve: add interactive mode

pulkit added a comment. For record: I left more comments about this patch. Mistakenly I did them on the phabricator commit instead of this review. https://phab.mercurial-scm.org/rHG5162753c4c14c143e6be011b89d6567b5ef50d66#51172 REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION

D6596: unshelve: add interactive mode

pulkit added inline comments. INLINE COMMENTS > navaneeth.suresh wrote in test-shelve.t:1354 > We only pass `-i` if we want to do the unshelve interactively again. It's not > mandatory to do that. After an interactive shelve, the stored remaining > shelve is same as a normal shelve. We can

D6596: unshelve: add interactive mode

pulkit added inline comments. INLINE COMMENTS > commands.py:6172 > + ('i', 'interactive', None, > + _('use interactive mode')), >('k', 'keep', None, Can you mark this as `EXPERIMENTAL` because right now we rebases the whole commit, which leads to conflicts in

D6645: continue: added logic for hg continue

pulkit added inline comments. INLINE COMMENTS > commands.py:1906 > +use --dry-run/-n to dry run the command. > +A new operation can be added to this by registering the operation and > +continue logic in the unfinishedstates list under statemod. This line should not be a part of user

D6430: rust-discovery: using from Python code

pulkit added a comment. I tested this series on our repository and found the following speedup in a case when there are 6k heads and 15k csets missing: Before: ! wall 3.281062 comb 3.28 user 3.20 sys 0.08 (best of 25) ! wall 4.941540 comb 4.94 user 4.86 sys

D6638: abort: removed labels argument from abortmerge()

pulkit added a comment. > However when merge is aborted there is no unresolved merge in progress and hence This needs to be reworded. > no labels are required. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6638/new/ REVISION DETAIL

D6596: unshelve: make unshelve accept files (issue6162)

pulkit added a comment. It will be nice to add information to commit message about how thing unshelving of a subset works. INLINE COMMENTS > shelve.py:961 > restorebranch(ui, repo, branchtorestore) > -_forgetunknownfiles(repo, shelvectx, addedbefore) > +if not

D6588: abort: added support for merge

This revision is now accepted and ready to land. pulkit added inline comments. pulkit accepted this revision. INLINE COMMENTS > pulkit wrote in hg.py:987 > Here the user is already running `hg merge --abort`, so we should not show > this message. > > Ideally, this `if` condition should not

D6588: abort: added support for merge

pulkit added inline comments. INLINE COMMENTS > hg.py:987 > +if stats.unresolvedcount: > +repo.ui.status(_("use 'hg resolve' to retry unresolved file merges " > + "or 'hg merge --abort' to abandon\n")) Here the user is already running `hg merge --abort`, so

D6588: abort: added support for merge

This revision now requires changes to proceed. pulkit added inline comments. pulkit requested changes to this revision. INLINE COMMENTS > pulkit wrote in hg.py:985 > We lost `return stats.unresolvedcount > 0` in this code movement. This is not yet addressed. REPOSITORY rHG Mercurial CHANGES

D6579: abort: added support for unshelve

pulkit added inline comments. INLINE COMMENTS > shelve.py:648 > +'please update to some commit\n')) > +return (state, opts) > + we can prevent returning opts here. One user does not need it and other one is passing by reference. REPOSITORY rHG Mercurial

D6579: abort: added support for unshelve

pulkit added a comment. The commit message is outdated. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc: mharbison72, pulkit, mercurial-devel

D6636: unshelve: changed Corruptedstate error msg from ui.warn to error.Abort

pulkit added a comment. > This is fone so as to avoid the return statement s/fone/done in flight INLINE COMMENTS > shelve.py:884 > shelvedstate.clear(repo) > -return > +raise error.Abort(_('could not read shelved state file, your > ' > +

D6588: abort: added support for merge

pulkit added inline comments. INLINE COMMENTS > hg.py:985 > +labels=labels) > +_showstats(repo, stats) > + We lost `return stats.unresolvedcount > 0` in this code movement. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION

D6579: abort: added support for unshelve

pulkit added inline comments. INLINE COMMENTS > shelve.py:628 > > -def unshelveabort(ui, repo, state, opts): > +def _loadshelvedstate(ui, repo, continuef=False, abortf=False, **opts): > +try: continuef and abortf can be read from opts, no need to pass them separately. Also, `opts` should

D6582: abort: added support for histedit

This revision is now accepted and ready to land. pulkit added inline comments. pulkit accepted this revision. INLINE COMMENTS > test-histedit-no-backup.t:63 > Enable config option: > + >$ cat >>$HGRCPATH

D6568: abort: added support for rebase

pulkit added inline comments. INLINE COMMENTS > rebase.py:1929 > +rbsrt = rebaseruntime(repo, ui) > +rbsrt._prepareabortorcontinue(isabort=True, backup=True, > + suppwarns=False) If I am correct, the call to abort from `hg rebase --abort`

D6567: abort: added support for graft

pulkit added a comment. > This logic is registered to the statesetection API as abortfunc. s/statesection/statedetection > graft currently supports --dry-run flag. This sounds ambiguous. Does this means `hg graft` supports `--dry-run` flag? IIUC, `hg abort` supports `--dry-run`

D6566: abort: added logic for of hg abort

pulkit added a comment. > hg abort currently supports --dry-run/-n flag only. > It is used to dry run hg abort We should specify what does dry run does i.e. it prints the operation which will be aborted if `hg abort` is run. INLINE COMMENTS > commands.py:139 > +def abort(ui, repo,

D6607: merge: disallow merge abort in case of an unfinished operation(issue6160)

pulkit added a comment. Also, can you add an entry about this to relnotes/next? INLINE COMMENTS > commands.py:4020 > +state = cmdutil.getunfinishedstate(repo) > +if state._opname != 'merge': > +raise error.Abort(_('cannot abort merge with %s in progress') % state

D6621: crecord: provide 'X' as a range-select mechanism

pulkit added a comment. Maybe this deserves and entry in relnotes/next as a followup. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6621/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6621 To: spectral, #hg-reviewers, pulkit Cc:

D6582: abort: added support for histedit

pulkit added a comment. In D6582#96537 , @taapas1128 wrote: > Okay I will do that. So should I remove `--no-backup` flag altogether from this series of patches because none of the commands support `--no-backup` then. Yes please remove.

D6582: abort: added support for histedit

pulkit added a comment. In D6582#96533 , @taapas1128 wrote: > @pulkit Yeah I know that but look at D3965 . histedit once did support `--no-backup` flag. It was dropped off for a different reason.

D6582: abort: added support for histedit

pulkit added a comment. Looking closely, the backup value comes from reading `ui.configbool('rewrite', 'backup-bundle')` instead of a `--no-backup` flag. It means we can get rid of the --backup flag to `hg abort` as there exists no such flag for histedit. REPOSITORY rHG Mercurial

D6608: graft: moved abortgraft and readgraft to cmdutil

pulkit added a comment. It will be nice to mention about our motivation to move code in the commit message. INLINE COMMENTS > cmdutil.py:3350 > + > +def _abortgraft(ui, repo, graftstate): > +"""abort the interrupted graft and rollbacks to the state before > interrupted We should drop

D6607: merge: disallow merge abort in case of an unfinished operation(issue6160)

pulkit added inline comments. INLINE COMMENTS > commands.py:4019 > if abort: > +for state in statemod._unfinishedstates: > +if state.isunfinished(repo) and state._opname != 'merge': We are performing the same loop in https://phab.mercurial-scm.org/D6566. How about

D6607: merge: disallow merge abort in case of an unfinished operation(issue6160)

pulkit added inline comments. INLINE COMMENTS > commands.py:4017 > +unfinishedstate = None > +for state in statemod._unfinishedstates: > +if state.isunfinished(repo): We should first check `if abort` before performing this loop instead of checking later. > commands.py:4019 > +

D6596: unshelve: make unshelve accept files (issue6162)

pulkit added inline comments. INLINE COMMENTS > shelve.py:842 > repo.setparents(tmpwctx.node(), nodemod.nullid) > newnode = repo.commit(text=shelvectx.description(), >extra=shelvectx.extra(), We can do filtering of files to unshelve

D6568: abort: added support for rebase

pulkit added inline comments. INLINE COMMENTS > rebase.py:1926 > > +def hgabortrebase(ui, repo, **opts): > +if opts.get('no_backup'): we can name this function abortrebase, no need to add hg in front. > rebase.py:1928 > +if opts.get('no_backup'): > +raise

D6567: abort: added support for graft

pulkit added a comment. > Tests have been shown as test-abort.t There is no such test now. :) INLINE COMMENTS > commands.py:2733 > > +def hgabortgraft(ui, repo, **opts): > +if opts.get('no_backup'): let's move this function to cmdutil.py. Should move _readgraftstate,

D6566: abort: added logic for of hg abort

pulkit added inline comments. INLINE COMMENTS > commands.py:135 > > +@command('abort', > +[('', 'no-backup', False, _('do not save backup copies of files')), Can you mark this command as experimental for now, we will be close to release soon. Marking this as experimental will help us

D6588: abort: added support for merge

pulkit added inline comments. INLINE COMMENTS > test-commit-unresolved.t:48 > +#if abortflag >$ hg merge --abort >abort: no merge in progress `hg abort` can be used. we can case the output here. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION

D6588: abort: added support for merge

pulkit added inline comments. INLINE COMMENTS > test-commit-unresolved.t:1 > +#testcases hgabort abortflag > + `abortcommand` will be better name instead of `hgabort` as that will match the way we named `abortflag` and will be more easier to realise the difference between them. REPOSITORY

D6588: abort: added support for merge

pulkit added inline comments. INLINE COMMENTS > test-commit-unresolved.t:45 >[255] > +#endif > + since there are only two cases, you can use `#else` here. > test-commit-unresolved.t:169 > > +#if abortflag >$ hg merge --abort We can put only the command in `if` and `else` part. The

D6597: rust-2018: switch hg-core and hg-cpython to rust 2018 edition

pulkit added a comment. Can you add link of mailing list thread in your commit message? Link from markmail or https://www.mercurial-scm.org/pipermail/mercurial-devel/. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6597/new/ REVISION DETAIL

D6479: shelve: first prototype of storing/restoring unresolved changes

pulkit added a comment. Since you did the nice work of moving shelve to core, how about rebasing this patch on tip of default with shelve in core? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6479/new/ REVISION DETAIL

D6588: abort: added support for merge

pulkit added inline comments. INLINE COMMENTS > hg.py:964 > +ui = repo.ui > +stats = abortmerge(ui, repo, labels=labels, asflag=True) > we can directly pass repo.ui here. No need to have a ui variable > hg.py:964 > +ui = repo.ui > +stats = abortmerge(ui, repo,

D6596: unshelve: make unshelve accept files (issue6162)

pulkit added inline comments. INLINE COMMENTS > navaneeth.suresh wrote in shelve.py:990 > will fix this in the next-to-next revision. To have this patch in a good state to be reviewed, it will be nice you take out the addition of interactive flag to the patch where you add that functionality.

D6579: abort: added support for unshelve

pulkit added inline comments. INLINE COMMENTS > shelve.py:630 > +if not state: > +try: > +state = shelvedstate.load(repo) Let's take this code out into a new function and reuse that function at both the places instead of duplicating code. REPOSITORY rHG Mercurial

D6553: shelve: move shelve extension to core

pulkit added inline comments. INLINE COMMENTS > commands.py:6207 > +with repo.wlock(): > +return shelvemod._dounshelve(ui, repo, *shelved, **opts) > + I just noticed that the function `_dounshelve()` still has `_` prefix. We generally have that to mark if functions are local to

D6582: abort: added support for histedit

pulkit added inline comments. INLINE COMMENTS > histedit.py:1891 > +if not state: > +state = histeditstate(repo) > +with repo.wlock(), repo.lock(): you need to set locks for the state too. Refer line 1685 in this file. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION

D6568: abort: added support for rebase

pulkit added a comment. Since we both are discussing and working on this, we can understand why this patch does what it does. It will be nice to have more explained commit descriptions which explain why we need to have an function independent of rebase-runtime-state object. This applies to

D6567: abort: added support for graft

pulkit added inline comments. INLINE COMMENTS > taapas1128 wrote in commands.py:2738 > @pulkit the wlock is activated later in `graft --abort` during cleanup. That > is same in the case of `hg abort` `_dograft()` is called after taking the lock. `_abortgraft` before taking the lock does a lot

D6566: abort: added logic for of hg abort

pulkit added inline comments. INLINE COMMENTS > commands.py:166 > + > +if nobackup: > +arglist = inspect.getargspec(abortstate.abortfunc)[0] Arguments for abort can increase in future. Rather pass the opts directly to abort functions for now and make related functions look for flag

D6596: unshelve: make unshelve accept files (issue6162)

pulkit added a comment. Didn't look at the actual implementation yet. INLINE COMMENTS > shelve.py:990 > +filesinclude = [] > +if interactive: > +for file in files: The interactive flag should work the same way as it work for revert, commit and shelve commands.

D6588: abort: added support for merge

pulkit added inline comments. INLINE COMMENTS > commands.py:151 > raise error.Abort(_('no operation in progress')) > +if abortstate._opname == 'merge': > +return hg.merge(repo, abort=True) let's refactor some code and create a function for merge abort instead of special

D6579: abort: added support for unshelve

pulkit added a comment. This patch needs to be rebased on tip of hg-committed as shelve is in core now. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6579/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6579 To: taapas1128, #hg-reviewers Cc:

D6567: abort: added functionality for graft

pulkit added inline comments. INLINE COMMENTS > commands.py:2738 > +'graft', fname='graftstate', clearable=True, stopflag=True, > +continueflag=True, abortfunc=_abortgraft, > +cmdhint=_("use 'hg graft --continue' or 'hg graft --stop' to stop") Directly calling `_abortgraft` is not

D6567: abort: added functionality for graft

pulkit added inline comments. INLINE COMMENTS > test-abort.t:2 > + > +TEST `hg abort` operation graft > + I think we can use cases functionality in existing tests to test `hg abort`. I mean, in one case alias `abort` to `graft --abort` and in other case it will be the normal abort

D6566: abort: first prototype of hg abort

pulkit added inline comments. INLINE COMMENTS > commands.py:147 > +if state.isunfinished(repo): > +abortstate = state > +break `return state.abortfunc(ui, repo, **opts)` instead of storing and calling that later. > commands.py:152 > +if

D6511: py3: hack around inconsistency of type of name passed to DNSQuestion

Closed by commit rHG8306b6c29add: py3: hack around inconsistency of type of name passed to DNSQuestion (authored by pulkit). This revision was automatically updated to reflect the committed changes. This revision was not accepted when it landed; it landed in state "Needs Review". REPOSITORY

D6510: py3: add r'' prefixes and do ('%d' % int) instead of str(int)

Closed by commit rHG683aeef12830: py3: add r prefixes and do (%d % int) instead of str(int) (authored by pulkit). This revision was automatically updated to reflect the committed changes. This revision was not accepted when it landed; it landed in state "Needs Review". REPOSITORY rHG

<    1   2   3   4   5   6   7   8   9   10   >