yuja added a comment.
I'm kinda -1 on this because of inconsistency with the 10-year codebase.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2010
To: indygreg, #hg-reviewers, pulkit, durin42
Cc: yuja, durin42, pulkit, mercurial-devel
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
Looks mostly good to me. I hesitated to fix minor nits in flight because
it's C.
INLINE COMMENTS
> bdiff.c:185
>
> +bool sliceintolist(PyObject *list, Py_ssize_t destidx,
> +
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> cmdutil.py:1593
> else:
> def write(s, **kw):
> fp.write(s)
We no longer need this `write()` wrapper because no labeling is required
yuja added inline comments.
INLINE COMMENTS
> bdiff.c:197
> + const char *text;
> + int i, start = 0;
> + Py_ssize_t nelts = 0, size;
Perhaps Py_ssize_t is preferred.
> bdiff.c:202
> + if (!PyArg_ParseTuple(args, "s#", , ))
> + goto abort;
> + if (!size) {
Here
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
These functions look quite similar. Perhaps we can extract a helper function
`f(repo, subset, x, rtypes)`.
INLINE COMMENTS
> remotenames.py:35
> registrar,
> +revset,
> +
yuja added a comment.
I'll drop this from hg-committed. Please make sure to touch your local
revision
before pulling.
INLINE COMMENTS
> commands.py:1674
> editor = ui.geteditor()
> ui.system("%s \"%s\"" % (editor, f),
>onerr=error.Abort,
yuja added a comment.
I'm thinking of adding a couple of utility functions as this sort of hack
is unavoidable around the codebase:
- `pycompat.maybebytestr(x)`
- make `repr(bytestr)` return `repr(bytes)[1:]`
- `pycompat.rapply(f, xs)` to recursively apply `f = maybebytestr` to
yuja accepted this revision.
yuja added a comment.
This revision is now accepted and ready to land.
Simple and huge improvement enough to push into the release.
Queued, thanks.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1934
To: indygreg, #hg-reviewers,
yuja accepted this revision.
yuja added a comment.
This revision is now accepted and ready to land.
This looks ugly, but seems okay. Since `sysstr()` decodes bytes as latin-1,
8-bit characters will be escaped by `\x`, not by `\u`.
REPOSITORY
rHG Mercurial
REVISION DETAIL
yuja added inline comments.
INLINE COMMENTS
> durin42 wrote in strip.py:218
> I'm unclear: are you asking to remove the b-prefix? Adding it was
> intentional, since eventually we're going to need to rewrite everything so we
> can ditch the module loader hack.
No. I mean it should use
yuja added a comment.
Perhaps it could be just `bytes`.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1903
To: durin42, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
yuja added a comment.
If we make `statusentry` stringifiable, we can do
__str__ = encoding.strmethod(__bytes__)
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1901
To: durin42, #hg-reviewers
Cc: yuja, mercurial-devel
yuja added inline comments.
INLINE COMMENTS
> strip.py:218
> # between the working context and uctx
> -descendantrevs = repo.revs("%s::." % uctx.rev())
> +descendantrevs = repo.revs(b"%d::." % uctx.rev())
> changedfiles = []
It should be
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
I'm skeptical if the generic write message will be actually used, so let's
revisit this later.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1871
yuja added a comment.
Queued the first two, thanks.
INLINE COMMENTS
> commands.py:5542
> +if ctx.obsolete():
> +obsfatemsg = obsutil._getfilteredreason(repo, ctxstr, ctx)
> +ui.warn("(%s)\n" % obsfatemsg)
If we use `_getfilteredreason()`
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
I'm 0 on this, but I have no reason against it.
INLINE COMMENTS
> blackbox.py:136
> +if self.debugflag:
> +self.log('debug', *msg, **opts)
> +
These
yuja added inline comments.
INLINE COMMENTS
> commands.py:3563
>
> -To undo an uncommitted merge, use :hg:`update --clean .` which
> +To undo an uncommitted merge, use :hg:`merge --abort .` which
> will check out a clean copy of the original merge parent, losing
Dropped ` .` in
yuja added a comment.
I think the frame details aren't important here, so making some lines optional
should be just fine.
Queued, thanks.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1858
To: indygreg, #hg-reviewers
Cc: yuja, mharbison72,
yuja added a comment.
Just for the record, explicit killdaemon isn't needed unless you want
to do something (e.g. checking server logs) after that. Daemons are
kill by the test runner.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1855
To:
yuja added inline comments.
INLINE COMMENTS
> pulkit wrote in verify.py:110
> Looks like we cannot use "%d" here as None can be possible value also and
> test-treemanifest.t has tests with None as a part of output.
Ugh, it smells, but let's just leave it for now.
REPOSITORY
rHG Mercurial
yuja added a comment.
Queued, thanks.
INLINE COMMENTS
> bookmarks.py:853
> +repo.ui.warn(_("bookmarking hidden changeset %s\n") % \
> + (', '.join(hiddenrevs)))
> marks.applychanges(repo, tr, changes)
FYI, it loops over bookmark names with the same `rev`, so
yuja added a comment.
Queued, thanks.
FWIW, it might be good idea to move `context._filterederror()` to obsutil.py
because context.py is painfully big.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1852
To: lothiraldan, #hg-reviewers, yuja
Cc:
yuja added inline comments.
INLINE COMMENTS
> subrepo.py:392
> if source.isabs():
> -return str(source)
> +return pycompat.bytestr(source)
> source.path = posixpath.normpath(source.path)
bytes()
> subrepo.py:399
> parent.path =
yuja added a comment.
In https://phab.mercurial-scm.org/D1694#31044, @durin42 wrote:
> I think it's probably okay for debug commands, those are pretty rare to use
as a function aren't they?
Yeah, it's okay, but why do we apply a different rule to debug commands?
If we take
yuja added inline comments.
INLINE COMMENTS
> subrepo.py:844
>
> -fp = self._repo.vfs("hgrc", "w", text=True)
> +fp = self._repo.vfs("hgrc", "wb", text=True)
> try:
No. Here we explicitly states that we want to open a file in "text" mode.
Needs something
yuja added a comment.
`bytes()` or `"%d"` is preferred. Can you send a follow up?
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1795
To: pulkit, #hg-reviewers, durin42
Cc: yuja, mercurial-devel
___
Mercurial-devel
yuja added inline comments.
INLINE COMMENTS
> obsutil.py:871
> +"""
> +successors = successorssets(unfilteredrepo, ctx.node())
> +fate = _getobsfate(successors)
Maybe it's okay to pass a "filtered" repo to this function as
`templatekw.showsuccessorssets()` does.
REPOSITORY
rHG
yuja added inline comments.
INLINE COMMENTS
> context.py:446
> +reason = obsutil._getfilteredreason(unfilteredrepo, ctx)
> +msg = _("hidden revision '%s' %s") % (changeid, reason)
> +else:
Not translatable.
Needs a table of
{'pruned': _("hidden revision '%s'
yuja added a comment.
I think it's new feature. Which Rust version should we support?
https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1170-2017-04-27
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1846
To: indygreg, #hg-reviewers
Cc:
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> bookmarks.py:859
> +if ctx.hidden():
> +repo.ui.warn(_("accessing hidden changeset %s\n") % ctx)
> +tgt = ctx.node()
yuja added inline comments.
INLINE COMMENTS
> kevincox wrote in main.rs:133
> `CString::new(env.python_exe.as_ref().as_bytes())`
That's more correct on Unix, but wouldn't work on Windows since
native string is UTF-16 variant (Rust) or ANSI (Python 2.7).
REPOSITORY
rHG Mercurial
REVISION
yuja accepted this revision.
yuja added a comment.
This revision is now accepted and ready to land.
Queued for stable. Thanks for fixing this issue.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1830
To: quark, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
yuja added inline comments.
INLINE COMMENTS
> indygreg wrote in Cargo.lock:1
> According to
> https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries
> (and other places I found on the Internet), `Cargo.lock` files under version
> control
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> bookmarks.py:856
> if rev:
> +warnm = "adding bookmarks to a hidden changeset"
> +repo = scmutil.unhidehashlikerevs(repo, [rev],
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> scmutil.py:1350
> +warnmessage += " %s\n"
> +repo.ui.warn(_(warnmessage) % revstr)
>
`_()` must wrap a string literal.
FWIW,
yuja accepted this revision.
yuja added a comment.
This revision is now accepted and ready to land.
> Are you thinking that it may be useful if the function in my test case is
instead a stand-alone script/binary?
No idea if it'll be useful, but there seemed no reason to narrow the scope
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
Needs some warning since a hidden revision may be revived?
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1813
To: pulkit, #hg-reviewers, yuja
Cc:
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
I'm drunk, but I'm sure this is good. Queued, thanks.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1810
To: pulkit, #hg-reviewers, yuja
Cc: yuja,
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
Suppose this is a kind of contrib code, I think it's good enough to accept.
Can you drop Cargo.lock file?
INLINE COMMENTS
> Cargo.lock:1
> +[[package]]
> +name = "aho-corasick"
yuja added a comment.
> However, it is quite large and I suspect it will take a bit more effort to
finish it.
Isn't that something like `dirstateTupleType`? If we had a native type, we
wouldn't
need https://phab.mercurial-scm.org/D1767,
https://phab.mercurial-scm.org/D1768, and
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> revlog.c:141
> + if (!value) {
> + PyErr_SetString(PyExc_ValueError,
> + "could not retrieve
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
> We hold off incrementing the version of the "parsers" extension
> because nothing in core relies on the new API yet.
It's really minor nit, but we have to increment the
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> revlog.c:1876
>
> + /* Initialize before argument-checking to avoid index_dealloc() crash.
> */
> + self->nullentry = Py_BuildValue("iiis#", 0, 0,
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> test-parseindex2.py:182
> +
> +idxa = [map(normvalue, a[0])]
> +idxb = [map(normvalue, b[0])]
Perhaps you mean `list(map(...))` instead of a list of one
yuja added a comment.
> Because we're now using a custom type and conforming to
> a yet-to-be-formally-defined interface, there's nothing
> preventing us from implementing a backed-by-C custom type
> for revlog entries.
Not reviewed the series yet, but can't we just start with a
yuja added inline comments.
INLINE COMMENTS
> indygreg wrote in streamclone.py:191
> Maybe.
>
> My reasoning here was that if the remote advertises phases data, then it is
> phases aware and the code below will ensure phases are adjusted
> appropriately. We certainly could special case the
yuja accepted this revision.
yuja added a comment.
This revision is now accepted and ready to land.
Queued, thanks. I've made asc/desc classes private as their constructor
behavior is weird.
> @staticmethod could be a simpler way to break cycles.
Subclassing is a better tool in
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> commands.py:5530
> +rev = ctx.rev()
> +if repo.filtername == 'visible-hidden':
> +ui.warn(_("updating to a hidden changeset %s\n") %
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
Can you update the other callers which pass `['']`?
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1760
To: pulkit, #hg-reviewers, yuja
Cc: yuja,
yuja added inline comments.
INLINE COMMENTS
> yuja wrote in commands.py:1283
> Maybe needs `if rev:`?
> Passing revspec=[''] seems not right.
Can you send a followup?
Passing `['']` works because `parse('')` raises ParseError, but
that doesn't seem legit.
REPOSITORY
rHG Mercurial
REVISION
yuja added inline comments.
INLINE COMMENTS
> scmutil.py:1322
> +
> +if not repo.filtername.startswith('visible'):
> +return repo
It seems better to use an explicit list of filternames,
i.e. `('visible', 'visible-hidden')`. I don't think we should introduce
new naming scheme,
yuja added a comment.
>> FWIW, if the cache don't include "unhidden" revisions, maybe we don't
>> need new filter names like 'visible-hidden"?
>
> Yep I agree, but I wanted be on the safe side to prevent unknown bugs by
using the same filtername.
Actually we have to use new
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> streamclone.py:191
> +# to draft. Then we apply the remote phases data.
> +if remotephases:
> +roots = [ctx.node() for ctx in
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> revsetlang.py:665
> +
> +_hashre = util.re.compile('[0-9a-fA-F]{1,40}')
> +
Oops, one more thing. Needs `$` to match whole word.
Test something like `('symbol',
yuja added a comment.
> Since revisions are dynamically unhidden, we can't cache the result
> to the (unfiltered) repo per filterename.
FWIW, if the cache don't include "unhidden" revisions, maybe we don't
need new filter names like 'visible-hidden"?
REPOSITORY
rHG Mercurial
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> repoview.py:157
> func = filtertable[filtername]
> -repo.filteredrevcache[filtername] = func(repo.unfiltered())
> +
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> repoview.py:196
> +# revs which are exceptions and must not be hidden
> +self._visibilityexceptions = visibilityexceptions
>
We have to use
yuja added inline comments.
INLINE COMMENTS
> pulkit wrote in scmutil.py:1345
> Sorry I don't get you. Check the filtername of the source repo for what?
repo = repo.filtered('served')
repo = unhidehashlikerevs(repo, ...)
repo.filtername == 'visible-hidden'
Does it make sense?
REPOSITORY
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> commandserver.py:454
> +util.osutil.unblocksignal(signal.SIGCHLD)
> +except (OSError, AttributeError):
> +pass
My two cents, add
yuja added inline comments.
INLINE COMMENTS
> commands.py:1283
> +rev = opts.get('rev')
> +repo = scmutil.unhidehashlikerevs(repo, [rev], 'nowarn')
> +ctx = scmutil.revsingle(repo, rev)
Maybe needs `if rev:`?
Passing revspec=[''] seems not right.
REPOSITORY
rHG Mercurial
yuja added a comment.
This should come before https://phab.mercurial-scm.org/D1733?
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1734
To: pulkit, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
Other than that, this one looks good.
INLINE COMMENTS
> scmutil.py:1345
> +
> +repo = repo.filtered('visible-hidden')
> +repo.addvisibilityexceptions(revs)
If `visible-` is
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> revsetlang.py:665
> +
> +hashre = util.re.compile('[0-9a-fA-F]{1,40}')
> +
Nit: `_hashre` to mark it a private.
> revsetlang.py:674
> +
> +for example for
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> repoview.py:145
> # from scratch (very slow).
> -filtertable = {'visible': computehidden,
> - 'served': computeunserved,
> -
yuja added inline comments.
INLINE COMMENTS
> logtoprocess.py:94
> +# and pager will wait for scripts to end if we don't do that
> +nullrfd = open(os.devnull, 'r')
> +nullwfd = open(os.devnull, 'w')
Better to close these file objects in the parent
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
> XXX-REVIEW: I tried making the test file compatible with chg but it seems
> flaky. The output is different when running only this test without
parallelism
> (rt
yuja added a comment.
> but could have other side-effects so it kept under a config knob.
My two cents, `experimental.*` might be better if the config knob exists as
a workaround for an unknown bug.
INLINE COMMENTS
> logtoprocess.py:66
> +if createconsole:
> +
yuja added a comment.
Nice. Queued, thanks.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1698
To: martinvonz, #hg-reviewers, yuja
Cc: mercurial-devel
___
Mercurial-devel mailing list
yuja added a comment.
Queued the first three patches, but I'm not certain about this. Sometimes we
do the reverse change for ease of calling command function as a plain
function.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1694
To: martinvonz,
yuja added a comment.
> Since we are calling them at each command level, it will be better to call
unhidehashlikerevs() in `scmutil.revrange|revsingle`.
I thought about that, but `scmutil.rev*()` would have to return new filtered
`repo`
object, which didn't seem nice. And mutating a
yuja added inline comments.
INLINE COMMENTS
> lothiraldan wrote in test-logtoprocess.t:33
> I tried making the file path unique for each call (by adding `\$(date +%s)`)
> but it doesn't seems to change anything in the stability unfortunately :(
Can't we just append to the log file and sort the
yuja added a comment.
In https://phab.mercurial-scm.org/D1679#28814, @phillco wrote:
> @yuja is that just because of the `_manifest` property cache, or are there
others as well?
Others, too. workingctx._parents() for example.
REPOSITORY
rHG Mercurial
REVISION DETAIL
yuja accepted this revision.
yuja added a comment.
This revision is now accepted and ready to land.
Queued, thanks.
In https://phab.mercurial-scm.org/D1679#28741, @phillco wrote:
> A better way might just be to use `None` for `self.wtcx` when rebasing on
disk,
Yeah, it might
yuja added a subscriber: mharbison72.
yuja added a comment.
In https://phab.mercurial-scm.org/D1426#28695, @lothiraldan wrote:
> Without `DETACHED_PROCESS`, no console is spawned either with `shell=False`
or `shell=True` **but** Mercurial will wait for the log-to-process script to
yuja added a comment.
In https://phab.mercurial-scm.org/D1678#28686, @pulkit wrote:
> @yuja am I headed in the right direction? (I am not sure about whether the
API's I changed are used by extensions or not)
Sort of, but I was thinking of a simpler one. Just parse specs twice,
yuja added a comment.
Looks good, but can you flag this as (API) change?
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1658
To: martinvonz, durin42, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> synthrepo.py:453
> if path not in changes:
> changes[path] = None
> break
Perhaps
yuja added a comment.
In https://phab.mercurial-scm.org/D1621#28387, @joerg.sonnenberger wrote:
> The bitset version has shown already that optimizing this is worthwhile and
can eliminate up to 10% of the total size of the transaction object.
IIRC, Jun said a plain intbitset
yuja added a comment.
In https://phab.mercurial-scm.org/D1621#28285, @joerg.sonnenberger wrote:
> This version should be committable. It introduces the necessary API for
isolating further changes and gives a small improvement already. I'll be
looking at provider a memory and time
yuja added a comment.
In https://phab.mercurial-scm.org/D1622#27981, @joerg.sonnenberger wrote:
> Like I said, this is primarily a proof of concept. I'm still playing with
different approaches
Ah, okay. I've added [PoC] to the subject so this won't be queued by mistake.
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
> Although this is definitely better for the clone case. intbitset does not
seem to implement segmented bitsets so the memory usage could in theory be
worse for other cases.
yuja added a comment.
> Original code: 485s / 584MB
> Set version: 501s / 582MB
> intbitset: 478s / 563MB
I'm not sure how to read this. Do we only get -2/584MB better space
consumption against the original simplest implementation?
REPOSITORY
rHG Mercurial
REVISION DETAIL
yuja added inline comments.
INLINE COMMENTS
> quark wrote in localrepo.py:575
> I guess `hg log -r HASH --hidden` would go through this code path so `pass`
> makes sense.
Maybe. My concern was `getvisibilityexceptions()` "seemed" not
working correctly in that case. I know unfiltered repo has
yuja added a comment.
>> How long should the visibilityexception be preserved?
>
> The visibility exceptions must be preserved until we calculate filteredrevs
for that filter. After adding the expections, we clear filteredrevcache to make
sure we calculate that again.
Sounds like
yuja added inline comments.
INLINE COMMENTS
> quark wrote in phases.py:236
> `baseset.__contains__` is slower than `set.__contains__` so the current code
> is faster.
Really? It's aliased to `self._set.__contains__`.
REPOSITORY
rHG Mercurial
REVISION DETAIL
yuja added a comment.
> I have spend a lot of time on this and I am unable to make this work
without making it class attribute because to calculate filterrevs,
unfilteredrepo is passed and I lose the reference to the object on which I
initially stored the visibilityexceptions. If I call
yuja accepted this revision.
yuja added a comment.
This revision is now accepted and ready to land.
Queued, thanks. Can you send follow-up patches to address trivial issues?
INLINE COMMENTS
> revlog.c:692
> for (i = 0; i < len; i++) {
> PyObject *phaseval;
>
Removed
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
Can't we apply `_updateexceptions()` out of the `revset.match*()`?
It's horrible that a revset query does mutate the repo.
# somewhere near scmutil.revrange()
tree =
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
> As _visibilityexceptions is defined as a class attribute, mutating it is
likely to mutate it for all instances of repoview,
Indeed.
INLINE COMMENTS
> localrepo.py:575
> +
yuja added inline comments.
INLINE COMMENTS
> main.rs:101
> +fn set_python_home(env: ) {
> +let raw = CString::new(env.python_home.to_str().unwrap())
> +.unwrap()
Perhaps we'll need a utility function for platform-specific cstr
conversion.
On Unix, the story is simple. We can use
yuja accepted this revision.
yuja added inline comments.
This revision is now accepted and ready to land.
INLINE COMMENTS
> setup.py:499
> +
> +content = ''.join([
> +'# this file is autogenerated by setup.py\n',
Made it bytes and queued, thanks.
REPOSITORY
rHG Mercurial
yuja added a comment.
In https://phab.mercurial-scm.org/D1577#26858, @quark wrote:
> Yes. I think the proxy class can have all states related to the filter. For
example, `unfi.filteredrevcache[filtername]` could be
`unfi._filteredrepotypes[typename + filtername]._revcache`.
yuja accepted this revision.
yuja added a comment.
This revision is now accepted and ready to land.
Queued, thanks.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1579
To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
yuja added a comment.
> So the cls here is a good candidate to attach that exception revs.
> (ex. cls._visibleexceptionrevs = set()).
Do you mean a proxy class will be defined per filter? e.g.
class visiblerepoview(repoview, repocls):
# impl specific to "visible"
yuja added a comment.
Can you send a follow-up to fix these issues?
INLINE COMMENTS
> uncommit.py:259
> +# identify the commit from which to unamend
> +curctx = repo['.']
> +
Probably better to query `curctx` after locks are taken.
> uncommit.py:280
> +extras =
yuja added subscribers: indygreg, quark, yuja.
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
@quark, @indygreg
Is there any practical problem other than the wrong class name?
IIUC, the composed type object shouldn't
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> debugcommands.py:729
> +ui.write("%c %s %10d %s%s\n" % (ent[0], mode, ent[2],
> + encoding.strtolocal(timestr), file_))
> for f in
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> cmdutil.py:828
> 'h': lambda: short(node),
> -'m': lambda: re.sub('[^\w]', '_', str(desc))
> +'m': lambda: re.sub('[^\w]', '_',
yuja added a comment.
Queued, but I doubt if `pureextra == old.extra()` would work with your
extension.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1576
To: martinvonz, #hg-reviewers, yuja
Cc: mercurial-devel
___
yuja added a comment.
Queued, thanks.
INLINE COMMENTS
> run-tests.py:2533
> +self.options.extra_config_opt.append(
> +'extensions.logexceptions=%s' %
> logexceptions.decode('utf-8'))
> +
Nit: better to use `_strpath()` here because extra_config_opt is
a list of
901 - 1000 of 1219 matches
Mail list logo
