Story #2986

"arv edit {uuid}" command, with behavior like "visudo"

Added by Tom Clegg about 5 years ago. Updated almost 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
06/20/2014
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
1.0

Subtasks

Task #3053: Download json, run $EDITOR, upload updatedResolvedPeter Amstutz

Task #3071: Review 2986-arv-editResolvedPeter Amstutz

Associated revisions

Revision cdd94fae
Added by Peter Amstutz about 5 years ago

Merge branch '2986-arv-edit' closes #2986

Revision 5f0b3d26 (diff)
Added by Peter Amstutz about 5 years ago

Suppress "Resource or subcommand '' is not recognized." message when running
arv with no parameters. refs #2986

History

#1 Updated by Tom Clegg about 5 years ago

  • Target version set to 2014-07-16 Sprint

#2 Updated by Peter Amstutz about 5 years ago

  • Assigned To set to Peter Amstutz

#3 Updated by Peter Amstutz about 5 years ago

  • Status changed from New to In Progress

#4 Updated by Brett Smith about 5 years ago

Reviewing d827740

  • In the refactoring, the semantics of arv pipeline have been changed. It's a subcommand that has its own subcommands, similar to arv keep, although right now it only accepts one (run). I think that behavior needs to be restored.
  • It would be nicely user-friendly if arv edit reported parse errors in the edited file and asked the user whether they wanted to retry the edit or abort. visudo does this, and it helps smooth over the typo-fixing process.
  • If the editor gives a nonzero exit code, I think arv edit should report a brief message about that and exit nonzero as well. This would match the behavior of other tools like git when they spawn editors.
  • Following the discussion we had with Ward a little while back, I think we should always prefer Oj to JSON from now on. Part of arv edit uses JSON.parse.
  • The --resources option has been removed when we generate the parser, which seems safe enough since it's basically just another --help, but other code still checks whether it's set. It'd be nice to clean those up.
  • The help message for arv edit does not use a consistent verb tense. The first sentence is a run-on sentence. There's a typo in "fetchs."
  • In other messages throughout, I think "Arvados" should always be capitalized.
  • If I try to arv edit a Collection, it will tell me that the object isn't an Arvados object. That may be strictly true, but I worry it could confuse newer users who don't understand the distinction.

Thanks.

#5 Updated by Peter Amstutz about 5 years ago

Brett Smith wrote:

Reviewing d827740

  • In the refactoring, the semantics of arv pipeline have been changed. It's a subcommand that has its own subcommands, similar to arv keep, although right now it only accepts one (run). I think that behavior needs to be restored.

Fixed.

  • It would be nicely user-friendly if arv edit reported parse errors in the edited file and asked the user whether they wanted to retry the edit or abort. visudo does this, and it helps smooth over the typo-fixing process.

Fixed, after I got bit by this before you even suggested it.

  • If the editor gives a nonzero exit code, I think arv edit should report a brief message about that and exit nonzero as well. This would match the behavior of other tools like git when they spawn editors.

Fixed.

  • Following the discussion we had with Ward a little while back, I think we should always prefer Oj to JSON from now on. Part of arv edit uses JSON.parse.

I actually thought we were supposed to be using multi-json or something. The plethora of json parsers for Ruby makes my head hurt.

  • The --resources option has been removed when we generate the parser, which seems safe enough since it's basically just another --help, but other code still checks whether it's set. It'd be nice to clean those up.

What "other code" are you referring to?

  • The help message for arv edit does not use a consistent verb tense. The first sentence is a run-on sentence. There's a typo in "fetchs."

Fixed.

  • In other messages throughout, I think "Arvados" should always be capitalized.

I only found one place that was the case, other strings containing "arvados" were not output strings.

  • If I try to arv edit a Collection, it will tell me that the object isn't an Arvados object. That may be strictly true, but I worry it could confuse newer users who don't understand the distinction.

Eh, collections aren't supposed to be editable anyway.

#6 Updated by Brett Smith about 5 years ago

Peter Amstutz wrote:

Brett Smith wrote:

  • The --resources option has been removed when we generate the parser, which seems safe enough since it's basically just another --help, but other code still checks whether it's set. It'd be nice to clean those up.

What "other code" are you referring to?

if global_opts[:resources] inside parse_arguments.

  • If I try to arv edit a Collection, it will tell me that the object isn't an Arvados object. That may be strictly true, but I worry it could confuse newer users who don't understand the distinction.

Eh, collections aren't supposed to be editable anyway.

Agreed. The most I was asking for is a message that specifically says that, rather than claiming it's not an Arvados object.

#7 Updated by Peter Amstutz about 5 years ago

  1. Fixed
  2. Fixed

b653edb

#8 Updated by Brett Smith about 5 years ago

b653edb looks good to merge to me. Thanks.

#9 Updated by Anonymous about 5 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 50 to 100

Applied in changeset arvados|commit:cdd94fae8d40ad6dcee3de5d36de2d89921fd12d.

Also available in: Atom PDF