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

(rebase) Support embedded go_proto_library #6030

Merged

Conversation

blorente
Copy link
Collaborator

@blorente blorente commented Feb 6, 2024

This is just a rebase and cleanup of this PR from @tingilee : #5567. The feature work is entirely theirs.

fixes #5538, #3328

@github-actions github-actions bot added product: GoLand GoLand plugin awaiting-review Awaiting review from Bazel team on PRs labels Feb 6, 2024
@blorente blorente mentioned this pull request Feb 6, 2024
3 tasks
@blorente blorente force-pushed the blorente/blorente/address-comments-oss branch from 0c5166d to 6a9ba65 Compare February 13, 2024 13:59
@blorente
Copy link
Collaborator Author

@mai93 @tpasternak could I have a review on this? I'll give co-author credit to @tingilee on the merge commit, but they don't have their branch open to pushes.

@blorente blorente force-pushed the blorente/blorente/address-comments-oss branch 2 times, most recently from 756316c to 37d8793 Compare March 1, 2024 10:10
@blorente
Copy link
Collaborator Author

blorente commented Mar 1, 2024

I'll solve the WORKSPACE conflicts, it just never ends so I'll wait for a review before solving them again.

@mai93 mai93 self-assigned this Mar 1, 2024
@tpasternak
Copy link
Collaborator

LGTM, thank you @blorente

The only concern is about the new dependencies we add (in WORKSPACE.bzlmod). @mai93 can you please confirm they're ok?

@mai93
Copy link
Collaborator

mai93 commented Mar 11, 2024

The only concern is about the new dependencies we add (in WORKSPACE.bzlmod). @mai93 can you please confirm they're ok?

I think since bazel-gazelle is part of bazelbuild it should be fine.

@tpasternak
Copy link
Collaborator

The only concern is about the new dependencies we add (in WORKSPACE.bzlmod). @mai93 can you please confirm they're ok?

I think since bazel-gazelle is part of bazelbuild it should be fine.

It's also google.golang.org/grpc

@tpasternak
Copy link
Collaborator

cc @mai93

@mai93
Copy link
Collaborator

mai93 commented Mar 13, 2024

google.golang.org/grpc

I'm not sure why it should be concerning? cc @blorente if you have more details about it

@tpasternak
Copy link
Collaborator

It's not concerning. It's just a new dependency so I always have concerns, sorry

@tpasternak
Copy link
Collaborator

Ok, so if there are no issues with dependencies (cc @mai93), then it LGTM. Btw is org_golang_google_grpc actually used anywhere?

@blorente blorente force-pushed the blorente/blorente/address-comments-oss branch from bd6a835 to 60c5ee5 Compare March 19, 2024 11:25
@blorente blorente force-pushed the blorente/blorente/address-comments-oss branch from 5e770b3 to e9dd859 Compare March 19, 2024 11:47
@blorente
Copy link
Collaborator Author

I've addressed the comments (and added a comment on why we import grpc). @mai93 @tpasternak , could you please take a look now?

@blorente blorente requested a review from mai93 March 19, 2024 11:54
@blorente blorente mentioned this pull request Mar 19, 2024
@blorente blorente merged commit c2b5d11 into bazelbuild:master Mar 19, 2024
6 checks passed
@github-actions github-actions bot removed the awaiting-review Awaiting review from Bazel team on PRs label Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product: GoLand GoLand plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolution fails for go files inline with proto files
4 participants