This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1016

repoview: remove incorrect documentation of the function
ClosedPublic

Authored by pulkit on Oct 11 2017, 5:44 PM.

Details

Reviewers
ryanmce
Group Reviewers
hg-reviewers
Commits
rHG586645e0589c: repoview: remove incorrect documentation of the function
Summary

In repoview.py, computeunserved() and computemutable() functions had the same
documentation. The documentation of computemutable() is wrong. I was unable to
write documentation for the function but it's better to not having the
documentation than having it wrong.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Oct 11 2017, 5:44 PM
dlax added a subscriber: dlax.Oct 12 2017, 3:20 AM
dlax added inline comments.
mercurial/repoview.py
104

Maybe update the docstring instead of removing?

ryanmce requested changes to this revision.Oct 12 2017, 7:27 AM
ryanmce added a subscriber: ryanmce.

I agree that removing is better than leaving bad docs, so I'm +1 on this, but as @dlax suggests, improving it would be even better.

If you don't have ideas on how to improve it, feel free to re-request review and I can queue this.

This revision now requires changes to proceed.Oct 12 2017, 7:27 AM
pulkit requested review of this revision.Oct 12 2017, 1:24 PM
In D1016#17086, @ryanmce wrote:

I agree that removing is better than leaving bad docs, so I'm +1 on this, but as @dlax suggests, improving it would be even better.
If you don't have ideas on how to improve it, feel free to re-request review and I can queue this.

I also agree with @dlax but unfortunately I don't have much ideas with this. Sorry. (re-requesting review)

mercurial/repoview.py
104

I am not much confident about whether I understand this part of code correctly or not. So it was hard for me updating the doc.

Closed by commit rHG586645e0589c: repoview: remove incorrect documentation of the function (authored by pulkit). · Explain WhyOct 13 2017, 10:27 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/repoview.py (3 lines)

Diff 2687

mercurial/repoview.py

first = min(cl.rev(n) for n in repo._phasecache.phaseroots[secret]) first = min(cl.rev(n) for n in repo._phasecache.phaseroots[secret])
revs = cl.revs(start=first) revs = cl.revs(start=first)
secrets = set(r for r in revs if getphase(repo, r) >= secret) secrets = set(r for r in revs if getphase(repo, r) >= secret)
return frozenset(hiddens | secrets) return frozenset(hiddens | secrets)
else: else:
return hiddens return hiddens
def computemutable(repo): def computemutable(repo):
"""compute the set of revision that should be filtered when used a server
Secret and hidden changeset should not pretend to be here."""
dlaxUnsubmitted
Not Done

Maybe update the docstring instead of removing?

dlax: Maybe update the docstring instead of removing?
pulkitAuthorUnsubmitted
Not Done

I am not much confident about whether I understand this part of code correctly or not. So it was hard for me updating the doc.

pulkit: I am not much confident about whether I understand this part of code correctly or not. So it…
assert not repo.changelog.filteredrevs assert not repo.changelog.filteredrevs
# fast check to avoid revset call on huge repo # fast check to avoid revset call on huge repo
if any(repo._phasecache.phaseroots[1:]): if any(repo._phasecache.phaseroots[1:]):
getphase = repo._phasecache.phase getphase = repo._phasecache.phase
maymutable = filterrevs(repo, 'base') maymutable = filterrevs(repo, 'base')
return frozenset(r for r in maymutable if getphase(repo, r)) return frozenset(r for r in maymutable if getphase(repo, r))
return frozenset() return frozenset()