TritonDataCenter / node-manta

Node.js SDK for Manta
75 stars 54 forks source link

mchmod issues with parseCmdOptions #370

Closed joyent-automation closed 4 years ago

joyent-automation commented 5 years ago

mchmod issues with parseCmdOptions

This PR was migrated-from-gerrit, https://cr.joyent.us/#/c/5026/. The raw archive of this CR is here. See MANTA-4594 for info on Joyent Eng's migration from Gerrit.

CR discussion

@kellymclaughlin commented at 2018-11-12T19:39:55

Patch Set 2:

(1 comment)

@kellymclaughlin commented at 2018-11-14T16:54:38

Patch Set 2:

(1 comment)

Here's a gist of the changes I made: https://gist.github.com/kellymclaughlin/84107991f2b9cd08f96947dd30ee9ab9. I ran the full test suite with these changes and everything passes, but adding some tests to exercise mchmod as part of this change set would be good too. I'll try to work on that over the next few days.

Patch Set 2 code comments
lib/options.js#92 @kellymclaughlin

This would fix the problem with mchmod, but it would also undo some previous fixes (see https://github.com/joyent/node-manta/issues/346) for other commands. We just have some inconsistency in behavior across some of the tools unfortunately.

I think the solution here is to do a fold (a.k.a reduce) over opts._args to find the valid paths in the provided arguments, assert that there is at least one provided path, and then proceed to call parseCmdOptions. That ought to work for all of the commands.