This is an archive of the discontinued Mercurial Phabricator instance.

phabricator: add a helper function to convert DREVSPECs to a DREV dict list
ClosedPublic

Authored by mharbison72 on Mar 5 2020, 11:42 AM.

Details

Summary

Prep work for allowing multiple DREVSPECs to various commands, and properly
validating the input.

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

mharbison72 created this revision.Mar 5 2020, 11:42 AM

This seems like something that can easily have a few simple unit tests

This seems like something that can easily have a few simple unit tests

The next patch attempts to provide coverage. I'm not sure how else to test this, because it wants to communicate with the server to resolve these values, so then it needs to be run as a command to hook up with the VCR infrastructure.

Alphare accepted this revision.Mar 17 2020, 4:10 AM

The next patch attempts to provide coverage. I'm not sure how else to test this, because it wants to communicate with the server to resolve these values, so then it needs to be run as a command to hook up with the VCR infrastructure.

I guess that's fine. I just wish we had more unit tests for things like utils that are self-contained (in theory) and not really subject to change.

pulkit accepted this revision.Mar 20 2020, 3:44 AM
This revision is now accepted and ready to land.Mar 20 2020, 3:44 AM
yuja added a subscriber: yuja.Mar 20 2020, 4:37 AM

+def _getdrevs(ui, stack, *specs):
+ """convert user supplied DREVSPECs into "Differential Revision" dicts
+
+ See `hg help phabread` for how to specify each DREVSPEC.
+ """
+ if len(*specs) > 0:

^^^^^^

Fixed bad argument expansion since I had to rebase this. Please let me
know if that's wrong.

yuja added a comment.Mar 20 2020, 4:47 AM

+def _getdrevs(ui, stack, *specs):
+ """convert user supplied DREVSPECs into "Differential Revision" dicts
+
+ See `hg help phabread` for how to specify each DREVSPEC.
+ """
+ if len(*specs) > 0:

^^^^^^

Fixed bad argument expansion since I had to rebase this. Please let me
know if that's wrong.

Never mind. Maybe specs is a list containing a single list, in which case,
the code is valid.

In D8232#124097, @yuja wrote:

+def _getdrevs(ui, stack, *specs):
+ """convert user supplied DREVSPECs into "Differential Revision" dicts
+
+ See `hg help phabread` for how to specify each DREVSPEC.
+ """
+ if len(*specs) > 0:

^^^^^^

Fixed bad argument expansion since I had to rebase this. Please let me
know if that's wrong.

Never mind. Maybe specs is a list containing a single list, in which case,
the code is valid.

It looks like a tuple of strings:

>>> print('specs type is %s' % type(specs))
specs type is <type 'tuple'>
>>> print('specs is %r' % (specs,))
specs is ('D1', 'D2', 'D3')

I copied this pattern from somewhere in Mercurial (file patterns look like they're handled in a similar way), but don't remember exactly where at this point.