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

Add go_repository rule for importing go packages. #96

Merged
merged 1 commit into from
Feb 10, 2017
Merged

Add go_repository rule for importing go packages. #96

merged 1 commit into from
Feb 10, 2017

Conversation

DrMarcII
Copy link
Contributor

@DrMarcII DrMarcII commented Feb 8, 2017

  • Remove redundancy from go import paths.

- Remove redundancy from go import paths.
@DrMarcII DrMarcII requested a review from joshbruning February 8, 2017 22:44
@mention-bot
Copy link

@DrMarcII, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jart to be a potential reviewer.

@jart
Copy link
Contributor

jart commented Feb 9, 2017

This change is already in the other pull request for testing, correct? If so, I'm still planning on patching that internally. Just have a few other pressing concerns at the moment. I'll be able to take that on in the next few days, and I'm happy to do it as part of the other PR.

@DrMarcII
Copy link
Contributor Author

DrMarcII commented Feb 9, 2017

Which other pull request are you talking about? If you mean the one for tensorflow, it is currently pinned to an earlier version of rules_webtesting, but this PR is just a cleanup thing.

No worries about reviewing this change. People on my team can take care of that (I probably should just go ahead and disable mention bot, as it hasn't been very useful).

@jart
Copy link
Contributor

jart commented Feb 9, 2017

Oh whoops. I thought this was on TensorFlow for some reason.

@achew22
Copy link
Member

achew22 commented Feb 9, 2017

What do you think about moving to the rules_go's new_go_repository? It uses Gazelle to automatically generate build files by parsing the Golang files and detecting their dependencies. If run inside your own repo it will even write your BUILD files for you so that you don't need to think about them any more.

@DrMarcII
Copy link
Contributor Author

DrMarcII commented Feb 9, 2017

Hmm, I hadn't looked at new_go_repository before.

It looks like it has a couple of issues:

  1. It uses an equivalent git_repository, which at least one of my target customers is not a fan of (@jart can add more context for why).
  2. I want my imports of external libraries to not have to include :go_default_library (which is what the alias in my generated BUILD file is for). I have already filed an issue w.r.t. this at Use package name instead of go_default_library to define a default library bazel-contrib/rules_go#265.

Additionally, I have played a little with Gazelle in my project and haven't yet figured out how to get it working correctly. It hasn't been a high priority for me, so I haven't dug too deep yet.

@DrMarcII DrMarcII requested a review from juangj February 10, 2017 22:38
@DrMarcII DrMarcII merged commit 5d3a3d7 into bazelbuild:master Feb 10, 2017
@DrMarcII DrMarcII deleted the update_go_deps branch February 10, 2017 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants