pulkit (Pulkit Goyal)
Spy

Projects

User does not belong to any projects.

User Details

User Since
Jun 28 2017, 8:54 PM (90 w, 5 d)

Recent Activity

Sat, Mar 23

pulkit committed rHG111de135fc76: branchcache: add attributes to track which nodes are verified.
branchcache: add attributes to track which nodes are verified
Sat, Mar 23, 11:42 AM
pulkit committed rHGb5511845f9d5: branchcache: have a hasnode function to validate nodes.
branchcache: have a hasnode function to validate nodes
Sat, Mar 23, 11:42 AM
pulkit closed D6157: branchcache: have a hasnode function to validate nodes.
Sat, Mar 23, 11:42 AM
pulkit closed D6156: branchcache: add attributes to track which nodes are verified.
Sat, Mar 23, 11:42 AM
pulkit closed D6152: branchcache: rename itervalues() to iterheads().
Sat, Mar 23, 11:40 AM
pulkit closed D6154: branchcache: introduce hasbranch().
Sat, Mar 23, 11:40 AM
pulkit committed rHGb137a6793c51: branchcache: make entries a private attribute.
branchcache: make entries a private attribute
Sat, Mar 23, 11:40 AM
pulkit committed rHG0bd730fbcc2b: branchcache: introduce hasbranch().
branchcache: introduce hasbranch()
Sat, Mar 23, 11:40 AM
pulkit closed D6151: branchmap: remove the dict interface from the branchcache class (API).
Sat, Mar 23, 11:40 AM
pulkit closed D6155: branchcache: make entries a private attribute.
Sat, Mar 23, 11:40 AM
pulkit committed rHG7546bf46bfcd: branchmap: drop branchcache.setdefault() (API).
branchmap: drop branchcache.setdefault() (API)
Sat, Mar 23, 11:40 AM
pulkit closed D6153: branchmap: drop branchcache.setdefault() (API).
Sat, Mar 23, 11:40 AM
pulkit committed rHG662ffdde5adf: branchcache: rename itervalues() to iterheads().
branchcache: rename itervalues() to iterheads()
Sat, Mar 23, 11:39 AM
pulkit committed rHG624d6683c705: branchmap: remove the dict interface from the branchcache class (API).
branchmap: remove the dict interface from the branchcache class (API)
Sat, Mar 23, 11:39 AM

Wed, Mar 20

pulkit added inline comments to D6163: copies: extract function for deciding whether to use changeset-centric algos.
Wed, Mar 20, 3:11 PM
pulkit closed D6161: tests: glob seconds in test-upgrade-repo.t.
Wed, Mar 20, 2:07 PM
pulkit committed rHG22ed63869835: tests: glob seconds in test-upgrade-repo.t.
tests: glob seconds in test-upgrade-repo.t
Wed, Mar 20, 2:07 PM
pulkit closed D6160: store: recommend using `hg debugrebuildfncache` if fncache is corrupted.
Wed, Mar 20, 2:07 PM
pulkit committed rHG3e7cfa17df5d: store: recommend using `hg debugrebuildfncache` is fncache is corrupted.
store: recommend using `hg debugrebuildfncache` is fncache is corrupted
Wed, Mar 20, 2:07 PM
pulkit committed rHGf9344d04909e: debugsparse: abort if the repository is not sparse instead of ui.status().
debugsparse: abort if the repository is not sparse instead of ui.status()
Wed, Mar 20, 2:07 PM
pulkit closed D6149: debugsparse: abort if the repository is not sparse instead of ui.status().
Wed, Mar 20, 2:07 PM
pulkit retitled D6160: store: recommend using `hg debugrebuildfncache` if fncache is corrupted from store: recommend using `hg debugrebuildfncache` is fncache is corrupted to store: recommend using `hg debugrebuildfncache` if fncache is corrupted.
Wed, Mar 20, 1:47 PM
pulkit created D6161: tests: glob seconds in test-upgrade-repo.t.
Wed, Mar 20, 1:43 PM
pulkit created D6160: store: recommend using `hg debugrebuildfncache` if fncache is corrupted.
Wed, Mar 20, 1:40 PM

Tue, Mar 19

pulkit added a comment to D6124: patch: include newline at EOF in help text for interactive patch.

Amended the following in flight:

Tue, Mar 19, 2:19 PM
pulkit added a comment to D6124: patch: include newline at EOF in help text for interactive patch.

test-check-code.t says hi.

Tue, Mar 19, 2:15 PM
pulkit accepted D6125: revert: option to choose what to keep, not what to discard.
Tue, Mar 19, 2:07 PM
pulkit accepted D6124: patch: include newline at EOF in help text for interactive patch.
Tue, Mar 19, 2:06 PM
pulkit added a comment to D6125: revert: option to choose what to keep, not what to discard.

I like what this patch does. I am not sure why this needs to be experimental since hg revert -i is not. Also, let's document this config option in hg help revert.

Tue, Mar 19, 11:07 AM
pulkit updated the diff for D6154: branchcache: introduce hasbranch().
Tue, Mar 19, 9:41 AM
pulkit created D6157: branchcache: have a hasnode function to validate nodes.
Tue, Mar 19, 9:41 AM
pulkit added a dependent revision for D6156: branchcache: add attributes to track which nodes are verified: D6157: branchcache: have a hasnode function to validate nodes.
Tue, Mar 19, 9:40 AM
pulkit created D6156: branchcache: add attributes to track which nodes are verified.
Tue, Mar 19, 9:40 AM
pulkit committed rHGb1bc6e5f5249: merge with stable.
merge with stable
Tue, Mar 19, 9:37 AM
pulkit added inline comments to D6005: uncommit: added interactive mode -i(issue6062).
Tue, Mar 19, 7:38 AM
pulkit added a comment to D6038: push: added clear warning message when pushing closed branches(issue6080).

@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?

Tue, Mar 19, 7:34 AM
pulkit created D6152: branchcache: rename itervalues() to iterheads().
Tue, Mar 19, 7:30 AM
pulkit created D6154: branchcache: introduce hasbranch().
Tue, Mar 19, 7:30 AM
pulkit created D6155: branchcache: make entries a private attribute.
Tue, Mar 19, 7:30 AM
pulkit added a dependent revision for D6154: branchcache: introduce hasbranch(): D6155: branchcache: make entries a private attribute.
Tue, Mar 19, 7:30 AM
pulkit added a dependent revision for D6153: branchmap: drop branchcache.setdefault() (API): D6154: branchcache: introduce hasbranch().
Tue, Mar 19, 7:30 AM
pulkit created D6153: branchmap: drop branchcache.setdefault() (API).
Tue, Mar 19, 7:30 AM
pulkit created D6151: branchmap: remove the dict interface from the branchcache class (API).
Tue, Mar 19, 7:30 AM
pulkit added a dependent revision for D6152: branchcache: rename itervalues() to iterheads(): D6153: branchmap: drop branchcache.setdefault() (API).
Tue, Mar 19, 7:30 AM
pulkit added a dependent revision for D6151: branchmap: remove the dict interface from the branchcache class (API): D6152: branchcache: rename itervalues() to iterheads().
Tue, Mar 19, 7:30 AM
pulkit added a comment to D6082: phabricator: add a `--branch` flag to `hg phabsend`.
In D6082#88981, @Kwan wrote:
In D6082#88851, @pulkit wrote:

@mharbison72 thanks for tips on adding test. Will add tests in next iteration.

I found that Differentials do have a branch field, maybe we can use that? https://secure.phabricator.com/source/phabricator/browse/master/src/applications/differential/customfield/DifferentialBranchField.php

Seems worth a shot. I don’t know anything about it, but presumably this would be rendered specially in the web UI, like the test plan, etc. That sounds better than as a follow up comment. It also seems natural enough that maybe it can be done unconditionally, instead of needing the argument.

Yeah, it shows in the Diff Detail pane, like here. Unfortunately I think it can be only set when using the creatediff endpoint, which is what I had to change my fork to do (though I could be wrong, the conduit docs are too sparse to be sure).

Tue, Mar 19, 7:24 AM
pulkit added a comment to D6147: discovery: move cl.hasnode outside of the for-loop.

@lothiraldan There is no significant performance benefit which I am able to notice. There is minor improvement of around 0.2 sec on our large repo but that is not stable.

Tue, Mar 19, 7:21 AM

Mon, Mar 18

pulkit committed rHGa920a9e1795a: store: error out if fncache does not ends with a newline.
store: error out if fncache does not ends with a newline
Mon, Mar 18, 7:35 PM
pulkit committed rHG6cad258e1348: tracked: add documentation about `--import-rules` flag.
tracked: add documentation about `--import-rules` flag
Mon, Mar 18, 7:35 PM
pulkit closed D6148: store: error out if fncache does not ends with a newline.
Mon, Mar 18, 7:35 PM
pulkit closed D6150: tracked: add documentation about `--import-rules` flag.
Mon, Mar 18, 7:35 PM
pulkit updated the summary of D6082: phabricator: add a `--branch` flag to `hg phabsend`.
Mon, Mar 18, 5:47 PM
pulkit updated the summary of D6082: phabricator: add a `--branch` flag to `hg phabsend`.
Mon, Mar 18, 5:36 PM
pulkit updated the diff for D6082: phabricator: add a `--branch` flag to `hg phabsend`.
Mon, Mar 18, 5:07 PM
pulkit added inline comments to D6005: uncommit: added interactive mode -i(issue6062).
Mon, Mar 18, 1:17 PM
pulkit created D6150: tracked: add documentation about `--import-rules` flag.
Mon, Mar 18, 10:27 AM
pulkit added a dependent revision for D6149: debugsparse: abort if the repository is not sparse instead of ui.status(): D6150: tracked: add documentation about `--import-rules` flag.
Mon, Mar 18, 10:27 AM
pulkit created D6149: debugsparse: abort if the repository is not sparse instead of ui.status().
Mon, Mar 18, 10:27 AM
pulkit created D6148: store: error out if fncache does not ends with a newline.
Mon, Mar 18, 9:57 AM
pulkit closed D5296: store: don't read the whole fncache in memory.
Mon, Mar 18, 8:29 AM
pulkit committed rHGa56487081109: store: don't read the whole fncache in memory.
store: don't read the whole fncache in memory
Mon, Mar 18, 8:29 AM
pulkit updated the diff for D5296: store: don't read the whole fncache in memory.
Mon, Mar 18, 7:32 AM
pulkit added a comment to D6147: discovery: move cl.hasnode outside of the for-loop.

The series looks good to me after a very quick review. I think a more in-deep review will be necessary.

@pulkit Do you have measured some performance improvement with this series?

Mon, Mar 18, 7:30 AM

Sun, Mar 17

pulkit created D6147: discovery: move cl.hasnode outside of the for-loop.
Sun, Mar 17, 11:49 AM
pulkit added a dependent revision for D6146: discovery: prevent deleting items from a dictionary: D6147: discovery: move cl.hasnode outside of the for-loop.
Sun, Mar 17, 11:49 AM
pulkit created D6146: discovery: prevent deleting items from a dictionary.
Sun, Mar 17, 11:49 AM
pulkit added a dependent revision for D6145: discovery: drop some unused sets: D6146: discovery: prevent deleting items from a dictionary.
Sun, Mar 17, 11:49 AM
pulkit created D6145: discovery: drop some unused sets.
Sun, Mar 17, 11:49 AM
pulkit added a dependent revision for D6144: discovery: prevent recomputing info about server and outgoing changesets: D6145: discovery: drop some unused sets.
Sun, Mar 17, 11:49 AM
pulkit created D6144: discovery: prevent recomputing info about server and outgoing changesets.
Sun, Mar 17, 11:48 AM
pulkit accepted D6038: push: added clear warning message when pushing closed branches(issue6080).
Sun, Mar 17, 11:03 AM
pulkit updated the diff for D5296: store: don't read the whole fncache in memory.
Sun, Mar 17, 10:54 AM

Sat, Mar 16

pulkit added a comment to D6098: wix: restore COPYING.rtf.
In D6098#89413, @pulkit wrote:

I applied this locally and did hg diff -c . --git --binary locally and the output is empty. Not sure what's broken here.

Maybe it is a Phabricator/integration bug? Try this:

# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1552069233 28800
#      Fri Mar 08 10:20:33 2019 -0800
# Node ID 577eab065a686f838bf13def7810d8d6f5f7f7ec
# Parent  7e95ade0f369d7509d04d6c0eefc06ca3d26c6e7
wix: restore COPYING.rtf

8427fea04017 accidentally blew away the content of this file.
As part of restoring the content, I updated the copyright year
to 2019.

Differential Revision: https://phab.mercurial-scm.org/D6098

diff --git a/contrib/packaging/wix/COPYING.rtf b/contrib/packaging/wix/COPYING.rtf
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..320dd93cce160f2950b9e47d0144efecfe40a1d6
GIT binary patch
literal 1687
zc$|$@-*4MC5Z-fu{11n|tSvG}Qsalr!vL>H8l#D0*bWK<e4!=Ex|JdolCohK{@-^;
z$%~5t+kk+vtmAz@?jwB=E|jT|#$#OaW|L;aOcbgt6JE76n>5L84x+Rsgtev*i6{+f
zQRbC&UWEjfSSt@xwS&2>r5PQMmlp?-TWuZffRS*jNW{h|WfHO0t;&YOQLOnWHp(a`
zgLUxa%b*GjFdW>ZgN*)$$?fPUwy1XB(G)kL%~R1xR|t*6G!rhaUS~H6t#zwIDqjB-
z9pz7-B2k|u@T6ScI+Pm3&+-<sBpLpaCFw0Z$q;DCze}w_ngVR7l=F;ndGWS(cxl8-
zg_qReAE;*_=%FO3;B6Up(y0;bZ40?HKsKNc4%cl^Ktc)fnuOQ3z;e%Dw5CuFhy(=X
zY7e%I4_~G)@Fn`BVb-oS=J1HQGG2l7(C|0DQU$E{vHYIV7d<-MTk0wUS~CKLED*LN
zlAa+u0rcIjfY`j3feAaX4R88DEcc*gc-uGVtXa~=#@UUN#FUZV#1()hHm+<#S}JNu
zPkY!Q8NF7}dR4ai3aA#oO_mSOFH0CtSMY5-pO2@@)rcxXW$CTjs;(2(jYeurnw&IY
z&(xpp=AR#^e*DMeak5-7`}@gqdbe1>{qr2gFdNU8lg}@Y<2lS;=CkL;UC(`Dz}VQF
zP1nkM8qRBru5(c~1eVLSR-}B#eI=*2fM8kK&S5~a>Ey6Ixo!3iH&r0PnnoCn<QFke
zK#)XuS6Qg#{xt4>_&;PKoLPFsMrn*B2$nNcQhgz7xl!~MO|IL5X)sWCjL1^os2uqN
zy~l#|QD$gcZfU-+Ej@EY`zACs^xT1+?xR4X;d(T3jT(tIVe@zA)ZqOaIFHcSKul{4
zokQCC7+p%8gh-kGuN7B2#R-r3kU5<fiWOJz4x-~*A0(P?1}xa}VL(?z2azh+RwyOv
zmh#p!{+ks+Q`}-tj?ei3`jQV<U7)Zeti0-{#a+(_RnehZ(_IC!*rF#kCm>>py(_lw
z&^y`3YN6DB9oRA+cs?zPwCdB&6{+=<cwG?uR*_M;!YyIT7_R?w6KESFNy5o9f6_c+
zu{0n`29f)%3ZeP+`g*t94K}77Shu;>T?2ppd(VZgcvQOJ+qu*<WFHO>tJ&TBaWef1
x@A?k)>Ao9yrIo>aklXs*@p#}p&g*axtrmHi!oO`e9*?JMy2GYlg`X}j{sP5+QltO?
Sat, Mar 16, 10:12 PM
pulkit closed D6030: store: move logic to check for invalid entry in fncache to own function.
Sat, Mar 16, 9:39 PM
pulkit committed rHGd7ef84e595f8: store: move logic to check for invalid entry in fncache to own function.
store: move logic to check for invalid entry in fncache to own function
Sat, Mar 16, 9:39 PM
pulkit added a comment to D5296: store: don't read the whole fncache in memory.
In D5296#88016, @yuja wrote:
  • self.entries = set(decodedir(fp.read()).splitlines()) + + self.entries = [] + totalsize = self.vfs.stat('fncache').st_size

I don't think totalsize has to be stat()ed. We can just loop over until
fp.read() reaches to EOF. It's unreliable to assume that the stat('fncache')
see the identical file to the fp open()ed before.

+ chunksize = (10 ** 6) # 1 MB
+ chunk = b''
+ chunksize = min(totalsize, chunksize)

Do we have any test covering totalsize > chunksize case?

Sat, Mar 16, 8:37 PM
pulkit updated the diff for D5296: store: don't read the whole fncache in memory.
Sat, Mar 16, 8:33 PM
pulkit accepted D5955: watchman: ignore some of watchman errors.
Sat, Mar 16, 7:19 PM
pulkit accepted D5954: watchman: add the possibility to set the exact watchman binary location.
Sat, Mar 16, 7:17 PM
pulkit accepted D6143: context: use wdirhex constant instead of calculating it.
Sat, Mar 16, 7:15 PM
pulkit added a comment to D6038: push: added clear warning message when pushing closed branches(issue6080).

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?

Sat, Mar 16, 7:10 PM
pulkit accepted D6127: split: use the new movedirstate() we now have in scmutil.
Sat, Mar 16, 7:03 PM
pulkit added inline comments to D6056: patch: stop aborting when add/rename/copy files on --interactive (issue5727).
Sat, Mar 16, 7:01 PM
pulkit added a comment to D2010: check-commit: allow foo_bar naming in functions.

Should we queue this patch or abandon it?

I'm for it, even though it leads to inconsistency. However, we may want to discuss ahead of time what our long-term plan for existing symbols is. Do we eventually want to remove that inconsistency? I took a quick look for examples where it seemed obviously not worth it to rename and it was harder to find good examples than I had expected. Perhaps bail_if_changed and extensions.wrap_function are some of the more frequently used. But most very commonly used functions seem to have short names already. So maybe even if we wanted to eventually make it consistent, it won't be as bad as people have feared? I still don't feel very strongly, but I wanted to highlight what it would mean in practice.

Sat, Mar 16, 6:28 PM

Fri, Mar 15

pulkit added inline comments to D6127: split: use the new movedirstate() we now have in scmutil.
Fri, Mar 15, 9:56 AM
pulkit added a comment to D6098: wix: restore COPYING.rtf.

I applied this locally and did hg diff -c . --git --binary locally and the output is empty. Not sure what's broken here.

Fri, Mar 15, 9:50 AM
pulkit added a comment to D6123: similar: add condition to avoid Zerodivisonerror in function _score() (issue6099).

Can you try to add a test for this?

Fri, Mar 15, 9:44 AM
pulkit added a comment to D2010: check-commit: allow foo_bar naming in functions.

Should we queue this patch or abandon it?

Fri, Mar 15, 9:41 AM
pulkit added inline comments to D6056: patch: stop aborting when add/rename/copy files on --interactive (issue5727).
Fri, Mar 15, 9:39 AM
pulkit added inline comments to D6038: push: added clear warning message when pushing closed branches(issue6080).
Fri, Mar 15, 9:37 AM
pulkit accepted D6133: rebase: fix crash with in-memory rebase and copies.
Fri, Mar 15, 9:34 AM
pulkit accepted D6132: test: demonstrate crash with in-memory rebase and copies.
Fri, Mar 15, 9:34 AM

Wed, Mar 13

pulkit added inline comments to D6123: similar: add condition to avoid Zerodivisonerror in function _score() (issue6099).
Wed, Mar 13, 4:54 AM
pulkit accepted D6121: chunkselector: fix typos in instructions when user reviews patch.
Wed, Mar 13, 4:52 AM
pulkit added a comment to D6005: uncommit: added interactive mode -i(issue6062).

Can you update the commit message to explain how this works? I think I saw somewhere else that it first uncommits everything and then does an interactive amend. Is that correct?

Wed, Mar 13, 4:32 AM

Mon, Mar 11

pulkit accepted D6119: uncommit: move _movedirstate() to scmutil for reuse.
Mon, Mar 11, 6:42 PM
pulkit accepted D6118: copies: remove dependency on scmutil by directly using match.exact().
Mon, Mar 11, 6:40 PM
pulkit accepted D6117: uncommit: convert _fixdirstate() into _movedirstate().
Mon, Mar 11, 6:39 PM
pulkit accepted D6120: scmutil: document matcher argument of movedirstate().
Mon, Mar 11, 6:31 PM

Fri, Mar 8

pulkit added a comment to D6082: phabricator: add a `--branch` flag to `hg phabsend`.

@mharbison72 thanks for tips on adding test. Will add tests in next iteration.

Fri, Mar 8, 5:30 PM