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

feat: add --json to compute add collaborator #469

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jheikes515
Copy link

What does this PR do?

Adds JSON support to sf env compute collaborator add, referenced in Elastic Services DX CLI Improvements for 240

What issues does this PR fix or reference?

See above.

@git2gus
Copy link

git2gus bot commented Jul 28, 2022

Git2Gus App is installed but the .git2gus/config.json doesn't exist.

@jheikes515 jheikes515 requested a review from saracope July 28, 2022 21:31
src/commands/env/compute/collaborator/add.ts Show resolved Hide resolved
src/commands/env/compute/collaborator/add.ts Show resolved Hide resolved
@@ -74,5 +72,8 @@ export default class ComputeCollaboratorAdd extends Command {
this.log(
'For more information about attaching Heroku add-ons to your compute environments, run $ heroku addons:attach --help.'
);
return {
Copy link
Member

Choose a reason for hiding this comment

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

If the user includes the --json flag, the error output should be in json also.

Try adding in:

if (shouldExitNonZero) {
      this.exit(1);
    }

Copy link
Member

Choose a reason for hiding this comment

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

Example here

if (shouldExitNonZero) {

src/commands/env/compute/collaborator/add.ts Show resolved Hide resolved
@@ -33,13 +41,25 @@ describe('sf env compute collaborator add', () => {
);
});

test
Copy link
Member

Choose a reason for hiding this comment

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

Also add in a failing testing for the --json flag

test
.stdout()
.stderr()
.nock('https://api.heroku.com', (api) => api.post('/salesforce-orgs/collaborators').reply(404, {}))
.command(['env:compute:collaborator:add', '-h', HEROKU_USER])
.catch((error) => {
expect(error.message).contains(`${herokuColor.heroku(HEROKU_USER)} does not exist.`);
expect(error.message).contains(`Couldn't find Heroku user ${herokuColor.heroku(HEROKU_USER)}.`);
Copy link
Member

Choose a reason for hiding this comment

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

See previous comment about this change. I'm not sure it's needed.

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