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 automatic module names (#584) #585

Merged
merged 3 commits into from Jan 7, 2020

Conversation

@comtel2000
Copy link
Contributor

comtel2000 commented Jan 7, 2020

works for me, BR, Joerg

Signed-off-by: Joerg Zimmermann <comtel2000@gmail.com>
@eclipse-milo-bot

This comment has been minimized.

Copy link
Contributor

eclipse-milo-bot commented Jan 7, 2020

Can one of the admins verify this patch?

@kevinherron

This comment has been minimized.

Copy link
Contributor

kevinherron commented Jan 7, 2020

I think bsd-parser will need one as well?

@comtel2000

This comment has been minimized.

Copy link
Contributor Author

comtel2000 commented Jan 7, 2020

you're right. Each 'jar' module requires a unique automatic name.
I missed bsd-parser-gson and bsd-parser-core

The bsd-parser-core should get a sub-package 'core' to be safe for JPMS.
I can rename/ or move it, but I don't know how many dependencies must be update too..
What do you think?

@kevinherron

This comment has been minimized.

Copy link
Contributor

kevinherron commented Jan 7, 2020

Hmm. I think that will be a breaking change, which would make it more appropriate for 0.4.0.

@kevinherron

This comment has been minimized.

Copy link
Contributor

kevinherron commented Jan 7, 2020

These packages have already started being reorganized on this branch in 0.4: https://github.com/eclipse/milo/tree/0.4-dtd-updates

@comtel2000

This comment has been minimized.

Copy link
Contributor Author

comtel2000 commented Jan 7, 2020

with the 0.4-dtd-updates branch it will be more complex. the 'core' only contains the resource xsd file and core classes are moved to the previous main module?
The old structure (with a additional sub-package) looks better to get it run with the java module system.

@kevinherron

This comment has been minimized.

Copy link
Contributor

kevinherron commented Jan 7, 2020

In that branch all the code in bsd-core is generated at build time under the package org.opcfoundation.opcua.binaryschema.

@comtel2000

This comment has been minimized.

Copy link
Contributor Author

comtel2000 commented Jan 7, 2020

ok, I need some time to check that. Once a module name is set, it's a pain in the a... to modify it later on.. :)

@kevinherron

This comment has been minimized.

Copy link
Contributor

kevinherron commented Jan 7, 2020

Ok. Any suggestions for how we can get this working for 0.3.7 without changing package names? Or should we wait and do the automatic module names in 0.4 instead?

I'm a little disoriented having just got back to work from a ~3 week holiday, but if I remember correctly I have a few branches off of dev/0.4 going and they're all pretty close to done, so a 0.4 release isn't too far in the future. Need to dig in this week and find out where everything stands.

comtel2000 added 2 commits Jan 7, 2020
Signed-off-by: Joerg Zimmermann <comtel2000@gmail.com>
Signed-off-by: Joerg Zimmermann <comtel2000@gmail.com>
@comtel2000

This comment has been minimized.

Copy link
Contributor Author

comtel2000 commented Jan 7, 2020

I guess this will work now for 3.7. For the 4.0 branches and JPMS compatibility the module root packages should be unique but as long as it's not switched to fully support JPMS (module-classpath) it works also with the current state (automatic-module-names).

@kevinherron

This comment has been minimized.

Copy link
Contributor

kevinherron commented Jan 7, 2020

Okay, thanks for doing this 👍

@kevinherron

This comment has been minimized.

Copy link
Contributor

kevinherron commented Jan 7, 2020

If this is ready and looks good to you I'll merge it.

@comtel2000

This comment has been minimized.

Copy link
Contributor Author

comtel2000 commented Jan 7, 2020

perfect, thx

@kevinherron kevinherron merged commit 8ddf376 into eclipse:master Jan 7, 2020
1 check passed
1 check passed
eclipsefdn/eca The author(s) of the pull request is covered by necessary legal agreements in order to proceed!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.