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

fix(travis): Correctly parse global_env from Travis v3 API #504

Merged
merged 5 commits into from
Sep 13, 2019

Conversation

jervi
Copy link
Contributor

@jervi jervi commented Sep 5, 2019

When the config section is returned as part of a build (which is unsupported in the Travis v3 API), global_env is a list of Strings. When it is returned as part of a job, global_env is instead a space separated String. And since env variable values can also contain spaces and equal signs, I had to create a little regex parser thingy.
This also fixes an issue in Deck where you can't add parameters to a Travis stage if .travis.yml contains anything in the env.global section.

When the `config` section is returned as part of a *build* (which is unsupported in the Travis v3 API), `global_env` is a list of Strings. When it is returned as part of a *job*, `global_env` is instead a space separated String. And since env variable values can also contain spaces and equal signs, I had to create a little regex parser thingy.
This also fixes an issue in Deck where you can't add parameters to a Travis stage if `.travis.yml` contains anything in the `env.global` section.
@jervi jervi force-pushed the fix_travis_property_parsing branch from 73e6b1a to b04836c Compare September 5, 2019 12:28
@jervi
Copy link
Contributor Author

jervi commented Sep 10, 2019

@spinnaker/reviewers PTAL

Copy link
Contributor

@emjburns emjburns left a comment

Choose a reason for hiding this comment

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

Looks good to me aside from one small comment, although I'm not a travis expert.

@SuppressWarnings("unchecked")
public void setGlobalEnv(Object globalEnv) {
if (globalEnv == null) {
this.globalEnv = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'd remove this initial null check & assignment. I don't think you need a null check before using instanceOf, and I think the variable will be null by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, you're right, and I've updated the PR 👍

@emjburns emjburns merged commit 093f121 into spinnaker:master Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants