Skip to content

SRAMP-421 S-ramp shell UploadArtifactCommand Add name and description#412

Closed
dvirgiln wants to merge 1 commit intoArtificerRepo:masterfrom
dvirgiln:SRAMP-421
Closed

SRAMP-421 S-ramp shell UploadArtifactCommand Add name and description#412
dvirgiln wants to merge 1 commit intoArtificerRepo:masterfrom
dvirgiln:SRAMP-421

Conversation

@dvirgiln
Copy link
Copy Markdown
Contributor

@dvirgiln dvirgiln commented May 5, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some problems here:

  1. Please do not re-order the arguments as that would potentially affect anyone with an existing s-ramp CLI command file. So please revert artifactType to be argument 1 and make name argument 2.
  2. Please update the Usage documentation (help text) for the command to indicate the new optional argument.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think is not possible to put the argument one (artifactType) as optional and then mandatory the argument two (the name).
How I can know that when the user introduce "upload fileToUpload parameter" which parameter is the value "parameter"? Is it the artifactType or it is the name... with aesh 0.53, using named args, this would have been solved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed - the name should be optional. It should use the filename if no name is provided. So in this way there is only one required argument (file path) and two optional (type, name).

@EricWittmann
Copy link
Copy Markdown
Contributor

I am closing this PR for now. We are deferring this change for a future when we change the CLI to use unordered arguments like this:

upload --file /path/to/file.ext --name "Name of Artifact" --type MyDocument

This will happen when we are able to use the latest AESH version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants