Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add compact option to site-uuid. #134

Merged
merged 1 commit into from
Sep 19, 2014

Conversation

eiriksm
Copy link
Contributor

@eiriksm eiriksm commented Apr 16, 2014

Hey guys!

Great work on this stuff!

OK, here goes:

Having psite-uuid outputting only the uuid is super useful for scripting, as pointed out in #133.

One can always bash hack around, as that issue shows. But adding an option does not break any current functionality and adds the features we all want (scripting abilities).

This patch exposes a --compact option to the command, which outputs only site uuid.

Also, the README uses psite-uuid and expects only the uuid to come back, which is not working (the example). Should be updated to, but waiting for some response on this before I send a pull request for that :)

@forssto
Copy link

forssto commented May 7, 2014

+1 to get this into the master branch. Would love to not have to do any magic to get the UUID pulled into a variable.

@joshkoenig
Copy link
Contributor

Sorry for lagging on this.

The only question is whether or not this is useful compared to our standard elsewhere of having output in JSON (our recommended route for scripting interactions; it's better to always pass data in some kind of buffered format) and/or whether we actually need the site name in the output under any other circumstance.

@forssto
Copy link

forssto commented May 7, 2014

JSON is great and all, but as these drush interactions are pretty often used in shell scripts, JSON is a bit of a pain to interact with. If there's a chance to just return a scalar value (as is the case in site-uuid), it should definitely have the option to do so.

@joshkoenig
Copy link
Contributor

Ok looking back the reason this got changed was to support multiple site-uuid inputs:

pantheon-systems/terminus@5001e22#diff-3c2b46b711ee764340a674d614c5aec8

@arshad - do you have an opinion on how best to handle this?

@eiriksm
Copy link
Contributor Author

eiriksm commented May 7, 2014

Just to chime in. This patch adds the --compact option, which could still exist in parallel with you guys ending up with json output as the default. It's just a feature added without breaking any functionality.

Also, if I remember correctly, the --json is what is used elsewhere in the code to get json back.

@shadcn
Copy link
Contributor

shadcn commented May 7, 2014

Seems to be working fine with multiple site-uuid as inputs as well. Does not break any existing functionality.

@mikevanwinkle mikevanwinkle merged commit 001df27 into pantheon-deprecated:master Sep 19, 2014
mikevanwinkle added a commit that referenced this pull request Sep 19, 2014
Merge branch 'eiriksm-site_uuid_compact'
@mikevanwinkle
Copy link
Contributor

I merged this. I think something along these lines could be useful for many commands, i.e. specifying a format that is friendly to bash scripting. Though eventually we might instead want to adopt a --format=bash, --format=pretty,--format=compact approach.

Thanks for the contribution @eiriksm

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.

5 participants