Page MenuHomePhabricator

command: add a cmdtype argument to registrar.command
AbandonedPublic

Authored by pulkit on Sep 2 2017, 11:37 AM.

Details

Reviewers
durham
Group Reviewers
hg-reviewers
Summary

The cmdtype argument defaults to registrar.cmdtype.UNRECOVERABLE_WRITE. This
argument will help us in telling whether a command is read only or write and
what kind of write. This information can be used in various ways.
Currently, we will use to information to decide what level of accessibility a
command can has on hidden commits based on it's type.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Sep 2 2017, 11:37 AM
pulkit edited the summary of this revision. (Show Details)Sep 2 2017, 1:39 PM
pulkit retitled this revision from directaccess: add a accesslevel argument to registrar.command to directaccess: add a hiddenlevel argument to registrar.command.
pulkit updated this revision to Diff 1580.
durham added a subscriber: durham.Sep 6 2017, 9:53 PM
durham added inline comments.
mercurial/registrar.py
150

I think an enum would be better here (UNRECOVERABLE_WRITE, RECOVERABLE_WRITE, READ_ONLY). Especially because I think people generally copy and paste the registrar decorators from other functions, and if we're just specifying numbers they are more likely to just reuse whatever value they copied.

yuja added a subscriber: yuja.Sep 7 2017, 10:25 AM
yuja added inline comments.
mercurial/registrar.py
150

nod. registrar.internalmerge() has a good example of pseudo enum.

pulkit planned changes to this revision.Sep 9 2017, 1:29 PM
pulkit edited the summary of this revision. (Show Details)Sep 11 2017, 5:56 PM
pulkit updated this revision to Diff 1730.
durham requested changes to this revision.Sep 12 2017, 12:05 PM
durham added inline comments.
mercurial/registrar.py
151

I was thinking an actual enum, to avoid potential typo's. For instance:

class hiddenlevel(object):
    UNRECOVERABLE_WRITE="unrecoverable"
    RECOVERABLE_WRITE="recoverable"
    READ_ONLY="readonly"
This revision now requires changes to proceed.Sep 12 2017, 12:05 PM
smf added a subscriber: smf.Sep 12 2017, 5:33 PM

Has there been discussion around extensions using this? Should there be discussion now? Specifically, I'm trying to figure out how external things will use this feature. Should it be a try/except? Or should modules set the access level before looking for a (potentially) hidden commit?

pulkit edited the summary of this revision. (Show Details)Sep 13 2017, 8:33 AM
pulkit retitled this revision from directaccess: add a hiddenlevel argument to registrar.command to command: add a cmdtype argument to registrar.command.
pulkit updated this revision to Diff 1778.
quark added subscribers: indygreg, quark.EditedSep 15 2017, 5:06 PM

I think @indygreg has some ideas around making "read-only" vs "writable" repo objects. I personally prefer whatever repo approach (having a field in repo, or use different types of repo objects).

Resend the series as D736, D737 and D738.

In D612#12426, @pulkit wrote:

Resend the series as D736, D737 and D738.

Can we discard this one then? It looks like those supersede this...

pulkit abandoned this revision.Sep 20 2017, 2:01 PM