search

bazeltools / bazel-deps

Generate bazel dependencies for maven artifacts
MIT License
250 stars 122 forks source link

Update README to fix broken instructions #249

Closed thundergolfer closed 5 years ago

thundergolfer commented 5 years ago

Instructions in README.md currently won't work because //:parse doesn't build the deploy JAR that gen_maven_deps.sh looks for.

johnynek commented 5 years ago

Thank you for the PR!

thundergolfer commented 5 years ago

Is it best to support both? I typically just use bazel run //:parse -- .... I think it'd be better if the README.md just called out one of the two ways, if either work.

I could change it to just direct users to call //:parse directly (as that will build things if it needs to). Seems simpler.

johnynek commented 5 years ago

Yeah. I’d recommend just using run directly now. Bazel run used to be pretty bad (it couldn’t use stdout/stdin)

thundergolfer commented 5 years ago

@johnynek 👍?

johnynek commented 5 years ago

Do you not need a — after the :parse to distribution bazel options from the options for the command?

I’m on a phone now so I can’t try but I want to double check that before merging.

Thank you for your help!

thundergolfer commented 5 years ago

Woops, should've run it instead of entering the command from memory! Copy-pasted in one I just ran from work comp. Works.