Skip to content

Conversation

@gssbzn
Copy link
Contributor

@gssbzn gssbzn commented Jul 29, 2020

Proposed changes

Jira ticket: CLOUDP-66895

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code

Further comments

Copy link
Contributor

@MihaiBojin MihaiBojin left a comment

Choose a reason for hiding this comment

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

LGTM, a few nits.

params:
working_dir: src/github.com/mongodb/mongocli
script: |
export SNYK_TOKEN=${snyk_token}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: export before set and also more exports below

Copy link
Contributor

Choose a reason for hiding this comment

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

also, missing ""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

before because it's a secret, it's an evergreen thing do move these kind of expansions before the sets also for expansions we usually don't use quotes("") but I can try it

set -Eeou pipefail
export GOROOT="${go_root}"
export PATH="./bin:$GOROOT/bin:$PATH"
Copy link
Contributor

Choose a reason for hiding this comment

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

is GOROOT needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left them because I'm not sure if snyk needs a working go env to run, if there's no GOROOT set go complains a lot, it has a default usually but that's not set on evergreen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, goroot is needed, test here without it

@gssbzn gssbzn merged commit 3bf3f2e into master Jul 30, 2020
@gssbzn gssbzn deleted the CLOUDP-66895 branch July 30, 2020 08:48
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.

3 participants