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

Expose maven_export #858

Merged
merged 1 commit into from
Mar 22, 2023
Merged

Expose maven_export #858

merged 1 commit into from
Mar 22, 2023

Conversation

sacsar
Copy link
Contributor

@sacsar sacsar commented Feb 20, 2023

This PR makes some of the changes referenced in #855. I am not convinced this is the best possible API to expose -- I basically made the minimal changes -- in particular, I think the lib_name parameter is a bit awkward (maybe it should be dep or library?) and I'm pretty sure the docstring could be further refined.

NOTE: I did run the doc script, but it resulted in no changes.

@google-cla
Copy link

google-cla bot commented Feb 20, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Collaborator

@shs96c shs96c left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for keeping the changes minimal. I'm curious to know what you think a better API would be, but we can discuss that on Slack :)

@shs96c
Copy link
Collaborator

shs96c commented Feb 23, 2023

@sacsar, before I can merge this PR, could you please sign the CLA? This comment contains a link to get you to the form you need to fill in.

Once the checks are green, I'll be happy to merge this PR.

@sacsar
Copy link
Contributor Author

sacsar commented Mar 3, 2023

Will do -- I'm trying to figure out if I should sign the CLA as myself or as my employer.

@shs96c
Copy link
Collaborator

shs96c commented Mar 3, 2023

Please take this with a large dose of "I am not a lawyer".

It depends on your employment contract with your employer. If they've made a land-grab for any IP you create while employed by them (even in your spare time), then I think it's best to sign the corp CLA. If you're working independently, and there's no clause in your employment contract attempting to lay claim to this PR, then signing as an individual is fine.

@sacsar
Copy link
Contributor Author

sacsar commented Mar 20, 2023

We decided that this is sufficiently close to my actual work that we should sign the "corporate CLA", which has been completed. I think we're now just in processing limbo, as I've updated the commit to have my work email address.

@shs96c shs96c merged commit 8e36eab into bazelbuild:master Mar 22, 2023
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

2 participants