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

Set GOPATH to the default directory if unset #19

Closed
wants to merge 5 commits into from
Closed

Set GOPATH to the default directory if unset #19

wants to merge 5 commits into from

Conversation

crazy-max
Copy link

@crazy-max crazy-max commented Sep 3, 2019

This PR should fix issues linked to GOPATH definition (#12 #14).
TravisCI also has pretty much this behavior while initializing Go with gimme and the official Go Docker image too.

@kjk
Copy link

kjk commented Sep 3, 2019

FYI: looks like you've inadvertently changed the line endings of the files, so unless the diff is told to ignore whitespace changes it's hard to tell what changed.

@crazy-max
Copy link
Author

crazy-max commented Sep 3, 2019

@kjk Yeah looks like tsc fucked up ^^
Scroll down to TS definition to look at my changes

Edit: Should be fine now

@crazy-max
Copy link
Author

@damccorm Some feedback on this ?

klingtnet added a commit to klingtnet/go-project-template that referenced this pull request Nov 28, 2019
For Go 1.13 no GOPATH is set by default but there is a PR that should
fix this: actions/setup-go#19
Anyways, as a workaround I explicitly set the GOPATH.
@crazy-max crazy-max requested review from hross and cdb February 7, 2020 08:28
@crazy-max crazy-max requested review from bryanmacfarlane and removed request for cdb and hross February 11, 2020 01:11
# Conflicts:
#	lib/installer.js
#	src/installer.ts
@bryanmacfarlane
Copy link
Member

Feedback:

  • the diff needs to be cleaned up so it's just what's changed. You may want to re-fork or something. I just did a massive rewrite and master is v2-beta and v1 is in releases/v1. If we do this, then v2 only.
  • I don't think this is the right thing to do for everyone. I think there should be a bool input whether to setGoPath and it should default to false. There's post 1.13 scenarios with modules where you absolutely don't want to do this and those scenarios are becoming more and more common as folks adopt modules which must have your code outside of GOPATH.
  • because of points above, I wonder whether we should do this. As time goes on, the feature is less needed.

@bryanmacfarlane
Copy link
Member

also, do an npm run pre-checkin which will reformat. Set line endings to auto. are you coding on windows?

@crazy-max
Copy link
Author

crazy-max commented Feb 11, 2020

@bryanmacfarlane

because of points above, I wonder whether we should do this. As time goes on, the feature is less needed.

Agree to close this issue looking at those points.

also, do an npm run pre-checkin which will reformat. Set line endings to auto. are you coding on windows?

Yeah I'm currently on Windows but will move back to Linux in few hours ;)

@crazy-max crazy-max closed this Feb 11, 2020
@bryanmacfarlane
Copy link
Member

I might also set some repo level git line level ending settings ...

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