Devvynpm volume ro support

[devvyn]

Slight change to splitting of the volume argument, to align with Docker's handling of the equivalent argument.

[aanand]

Thanks! Looks good.

1. Could we get an integration test?
2. The documentation for volumes needs updating.
3. I'd rather split_volume always returned a tuple of the same length, with the last part always being either ro or rw.
4. split_volume's docstring needs updating.
5. Could you squash it into one commit?
6. Commits need to be signed off.

[devvyn]

Ha ha, I would like to apologize: I meant to practice this on my fork
before submitting seriously, but I failed to notice whose repository I'd
applied the push to.

By the way, I think there is an issue on file (#260
#260) which applies and another issue
(#363 #363) which touches on fully
supporting all Docker arguments, which would include volume permissions. If
I can manage to wrangle git to do this properly, I will try this again in
reference to one or both issues.

Thanks for your helpful response. This will certainly help me succeed in my
next attempted pull request.

Devvyn Murphy
OTV Technologies Ltd.
2915B Faithfull Ave
Saskatoon, SK S7K 8E8
otvtech.com
T:1 (306) 374-0607, 283

M:+13068802322

On Mon, Aug 25, 2014 at 5:53 PM, Aanand Prasad notifications@github.com
wrote:

Thanks! Looks good.

1. Could we get an integration test?
2. The documentation for volumes needs updating.
3. I'd rather split_volume always returned a tuple of the same length,
   with the last part always being either ro or rw.
4. split_volume's docstring needs updating.
5. Could you squash it into one commit?
6. Commits need to be signed off
   https://github.com/docker/fig/blob/master/CONTRIBUTING.md#sign-your-work
   .

Reply to this email directly or view it on GitHub
#444 (comment).

[dnephin]

@devvynm if you're interested, I sent in a PR yesterday (#442) which adds this support along with additional validation and unit tests

[devvyn]

@dnephin Very happy to hear that. Thanks for letting me know.