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

Feature/batch script execution #150

Merged
merged 3 commits into from Jun 3, 2018

Conversation

darvin
Copy link
Contributor

@darvin darvin commented Jun 3, 2018

@darvin
Copy link
Contributor Author

darvin commented Jun 3, 2018

should expreduce/resources.go be gitignored?..

@corywalker
Copy link
Owner

Hi @darvin ,

This was initially the case, but after releasing 0.1, I noticed that people were having issues with the go generate steps. This meant they could not easily "go run expreduce.go". In the end, cznic and I decided to include generated Go source in the repo. Context here:

#40

You're right that this will cause merge conflicts to happen often on resources.go, something I did not consider. Perhaps a simple fix would be to have a separate resources.go for each module, reducing the chance of this. Optionally an idea would not be to have this resources.go and somehow load the files directly. Not sure if there is a standard way to do this with Go packages.

For now I'd recommend merging in changes from master and then regenerating resources.go. If it gets unmanageable we can implement one of the above solutions, or even gitignore resources.go.

-Cory

@corywalker
Copy link
Owner

The changes in this PR look great though. Thanks for this change. I can merge after you address the conflict on resources.go.

@darvin darvin force-pushed the feature/batch_script_execution branch from 1f4bf55 to 2f48d8e Compare June 3, 2018 02:19
@darvin
Copy link
Contributor Author

darvin commented Jun 3, 2018

addressed

@darvin
Copy link
Contributor Author

darvin commented Jun 3, 2018

offtopic, but have you considered using dep for dependency management/build system? I'm super new to golang, but it seems like something that would solve this problem (and much more)

edit: maybe not, I just would imagine package manager would have "pre install" step where you can generate things

@corywalker corywalker merged commit fd5082e into corywalker:master Jun 3, 2018
@corywalker
Copy link
Owner

I'm reading up on dep. It does seem to be used by some popular Go projects now. Not sure if it would help with the generated code issue, but it could help with managing versions of dependencies.

@cznic
Copy link
Contributor

cznic commented Jun 3, 2018

Suggesting vgo instead.

@darvin
Copy link
Contributor Author

darvin commented Jun 3, 2018

(I have no opinions on dep specifically, im total complete Go noobie :) )

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