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

Integrate apply command #175

Merged
merged 9 commits into from Jun 24, 2020
Merged

Integrate apply command #175

merged 9 commits into from Jun 24, 2020

Conversation

bmonkman
Copy link
Member

Various changes to get the apply command working including passing more required env vars in and correcting some of the paths.

Stream apply output
Changed apply test to compare a file rather than output
Changed prompts to use til for appending env
Added handling for relative paths during execution
Append env from outside zero when running commands inside
Changed to execute makefile in module dir, not project dir
@@ -50,6 +51,35 @@ func (p ProjectCredentials) Unmarshal(data []byte) error {
return nil
}

// AsEnvVars marshals ProjectCredential as a map of key/value strings suitable for environment variables
func (p ProjectCredential) AsEnvVars() map[string]string {
Copy link
Member Author

Choose a reason for hiding this comment

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

@davidcheung This should help with #173

@bmonkman bmonkman closed this Jun 23, 2020
@bmonkman bmonkman reopened this Jun 23, 2020
@bmonkman
Copy link
Member Author

Weird, dunno how this got closed. Must have been something I did in zenhub. Sorry.

@@ -0,0 +1,19 @@
name: sample_project

context:
Copy link
Contributor

@dtoki dtoki Jun 24, 2020

Choose a reason for hiding this comment

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

According to the new zero-project.yaml spec do we still support the context: field?

Comment on lines +23 to +25
if strings.Trim(configPath, " ") == "" {
exit.Fatal("config path cannot be empty!")
}
Copy link
Contributor

@dtoki dtoki Jun 24, 2020

Choose a reason for hiding this comment

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

Would it be better to return the error in this case and let the caller throw the error for consistency like initin #130?

Copy link
Contributor

@dtoki dtoki Jun 24, 2020

Choose a reason for hiding this comment

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

Nvm, I just realized your comment was specific to if I wanted to test the error case so I guess that doesn't apply here because it's not a standard we're enforcing.

@bmonkman bmonkman merged commit 29d07b7 into master Jun 24, 2020
@bmonkman bmonkman deleted the integrate-apply-command2 branch June 24, 2020 17:02
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.

None yet

3 participants