Project

General

Profile

Actions

Idea #2986

closed

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

Added by Tom Clegg over 10 years ago. Updated over 10 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
06/20/2014
Due date:
Story points:
1.0

Subtasks 2 (0 open2 closed)

Task #3053: Download json, run $EDITOR, upload updatedResolvedPeter Amstutz06/20/2014Actions
Task #3071: Review 2986-arv-editResolvedPeter Amstutz06/20/2014Actions
Actions #1

Updated by Tom Clegg over 10 years ago

  • Target version set to 2014-07-16 Sprint
Actions #2

Updated by Peter Amstutz over 10 years ago

  • Assigned To set to Peter Amstutz
Actions #3

Updated by Peter Amstutz over 10 years ago

  • Status changed from New to In Progress
Actions #4

Updated by Brett Smith over 10 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.

Actions #5

Updated by Peter Amstutz over 10 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.

Actions #6

Updated by Brett Smith over 10 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.

Actions #7

Updated by Peter Amstutz over 10 years ago

  1. Fixed
  2. Fixed

b653edb

Actions #8

Updated by Brett Smith over 10 years ago

b653edb looks good to merge to me. Thanks.

Actions #9

Updated by Anonymous over 10 years ago

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

Applied in changeset arvados|commit:cdd94fae8d40ad6dcee3de5d36de2d89921fd12d.

Actions

Also available in: Atom PDF