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 documentation for GoCmdEnv #207

Merged
merged 2 commits into from
Dec 12, 2018
Merged

Conversation

ihgann
Copy link
Contributor

@ihgann ihgann commented Dec 12, 2018

GoCmdEnv documentation was a copy of the verbose mode documentation - this change specifies the intention of the -gocmd flag.

`GoCmdEnv` documentation was a copy of the verbose mode documentation
- this change specifies the intention of the `-gocmd` flag.
@ihgann
Copy link
Contributor Author

ihgann commented Dec 12, 2018

I had a further question that might be better tracked as an issue - I use my magefile with some specific targets that utilize the go runtime, and in particular it's a bit confusing that setting -gocmd does not actually persist when I invoke mg.GoCmd() within my own magefile.

That is to say, -gocmd specifically only applies to compiling the magefile, but this seems misleading. I think if -gocmd is applied, then the magefile ought to be run with var MAGEFILE_GOCMD set as well. That doesn't apply to this change per se, but I only noticed this documentation note because I was confused by this behaviour.

Copy link
Member

@natefinch natefinch left a comment

Choose a reason for hiding this comment

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

Thanks!

@natefinch
Copy link
Member

That's a good point about it not getting passed through. Definitely worth an issue and something that is easy to fix.

@natefinch natefinch merged commit 4d83845 into magefile:master Dec 12, 2018
@ihgann
Copy link
Contributor Author

ihgann commented Dec 13, 2018

@natefinch Cool - maybe I can take a stab at it :)

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