Skip to content

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
@@ -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) {

@@ -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