Page MenuHomePhabricator

registrar: add a enum 'cmdtype' for the type of the command
AbandonedPublic

Authored by pulkit on Sep 13 2017, 8:33 AM.

Details

Reviewers
durham
yuja
Group Reviewers
hg-reviewers
Summary

This patch adds a new enum 'cmdtype' which will tell about the type of the
command that whether it's a read only command, it's recoverable write command or
an unrecoverable write command.

This will be used in deciding which level of hidden access a command can has.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Sep 13 2017, 8:33 AM

I'm holding off on accepting this because I'd like to see an example before it is committed.

Since Python 2 doesn't have real enums and Mercurial tends to shy away from object-oriented programming, I'm -0 on the use of a class here. Module-level variables with a common prefix would accomplish the same thing.

durham accepted this revision.Sep 14 2017, 8:30 PM
durham added a subscriber: durham.

I think the name could be better, but that can be bikeshed. Stamping my approval for the concept and pattern.

mercurial/registrar.py
148

cmdtype might be overly vague, since I could imagine a number of classifications it could mean. Maybe "cmdwritetype"?

yuja requested changes to this revision.Sep 15 2017, 9:40 AM
yuja added a subscriber: yuja.

To make it less controversial, I would move these constants to registrar.command
class and rename them to lowercaseconstants. The registrar provides semi-public
API, which should be consistently named.

I think this should be folded to the other registrar patch, and sent with the patches
which actually change the behavior depending on the cmdtype value. So marked as
change requested.

This revision now requires changes to proceed.Sep 15 2017, 9:40 AM
pulkit abandoned this revision.Sep 19 2017, 9:39 PM

Send the series as D736, D737 and D738.