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 the generated source folders to Eclipse classpath entries #596

Closed
wants to merge 3 commits into from

Conversation

jdneo
Copy link

@jdneo jdneo commented Aug 11, 2022

Signed-off-by: sheche sheche@microsoft.com

- add the generated source folders to Eclipse classpath entries. Due to
  a limition of Buildship, we have to create those folders beforehand,
  see: eclipse-buildship/buildship#1196

Signed-off-by: sheche <sheche@microsoft.com>
@google-cla
Copy link

google-cla bot commented Aug 11, 2022

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.

@ejona86
Copy link
Collaborator

ejona86 commented Aug 16, 2022

We've been in the process of reworking things so the explicit IDE configuration should be less necessary. I'd like to put this on hold a bit until some of the rework is merged and then I'd like you to re-test. What I expect in the future is the generated code should work fine without IDE configuration, but we may still need IDE configuration for .proto files.

@jdneo
Copy link
Author

jdneo commented Aug 17, 2022

Thank you for the information @ejona86!

To share more background: I'm working on Java language support in VS Code. We received a few feedbacks that VS Code could not recognize the source output directories from protobuf. That's why this PR is raised.

Is it possible to share more information about the generated code should work fine without IDE configuration? Does that mean that the protobuf plugin will generate the sources directly into the existing sourcesets?

@rougsig
Copy link
Collaborator

rougsig commented Aug 17, 2022

The Protobuf plugin will add the generated folders to the existing source sets. Just like you do it for eclipse, but through the vanilla gradle api, without eclipse plugin.

@jdneo
Copy link
Author

jdneo commented Aug 17, 2022

Thank you @rougsig.

What I did is to add those protobuf generated folders under the build folder to the Eclipse classpath entries.

If protobuf plugin can generate java source files into existing source sets, then I guess we can forget about the works in this PR. Since existing source sets should already contained in the classpath entries.

Could you please let me know once the rework is done? I'm glad to do some tests in VS Code and Eclipse. :)

@rougsig
Copy link
Collaborator

rougsig commented Aug 17, 2022

I'll plan to do this week, most likely it will be either Friday or Sunday. Can you create commits with tests? I will start working with them.

@jdneo
Copy link
Author

jdneo commented Aug 17, 2022

What kind of tests would you like to have?

@rougsig
Copy link
Collaborator

rougsig commented Aug 17, 2022

We need to make sure that vscode, idea, eclipse have generated code in their models (classpath). Like test in this PR.

@jdneo
Copy link
Author

jdneo commented Aug 17, 2022

I see. In the backend, VS Code and Eclipse share the same model. If it works in Eclipse, so do VS Code.

So, should I create another PR only contains test code? Or just leave as it is now?

@rougsig
Copy link
Collaborator

rougsig commented Aug 17, 2022

I see. In the backend, VS Code and Eclipse share the same model. If it works in Eclipse, so do VS Code.

Perfect!

So, should I create another PR only contains test code? Or just leave as it is now?

As you wish, I can cherry-pick the test.

Signed-off-by: sheche <sheche@microsoft.com>
@jdneo
Copy link
Author

jdneo commented Aug 18, 2022

As you wish, I can cherry-pick the test.

Thank you, I'll leave it here then.

@rougsig rougsig mentioned this pull request Aug 21, 2022
Signed-off-by: Sheng Chen <sheche@microsoft.com>
@jdneo
Copy link
Author

jdneo commented Aug 31, 2022

Close since we have #590 already.

@jdneo jdneo closed this Aug 31, 2022
@jdneo jdneo deleted the cs/eclipse branch August 31, 2022 01:34
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.

3 participants