Skip to content

Comments

[MJAR-260] Fail on invalid automatic module name#3

Merged
sormuras merged 1 commit intomasterfrom
MJAR-260-Automatic-Module-Name
Dec 1, 2018
Merged

[MJAR-260] Fail on invalid automatic module name#3
sormuras merged 1 commit intomasterfrom
MJAR-260-Automatic-Module-Name

Conversation

@sormuras
Copy link
Contributor

@sormuras sormuras commented Nov 16, 2018

@sormuras sormuras self-assigned this Nov 16, 2018
eolivelli
eolivelli requested changes Nov 16, 2018
View reviewed changes
Copy link

@eolivelli eolivelli left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I like this 'feature'

@sormuras sormuras force-pushed the MJAR-260-Automatic-Module-Name branch from 6cd74eb to 736e16c Compare November 16, 2018 15:23
eolivelli
eolivelli approved these changes Nov 16, 2018
View reviewed changes
Copy link

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Lgtm

Awesome!

@eolivelli
Copy link

eolivelli commented Nov 16, 2018

Seems there is a failure
https://builds.apache.org/job/maven-box/job/maven-jar-plugin/job/MJAR-260-Automatic-Module-Name/2/

@eolivelli
Copy link

eolivelli commented Nov 16, 2018

I wonder if we should fix other plugins, namely the shade plugin

@sormuras
Copy link
Contributor Author

sormuras commented Nov 16, 2018 •
edited
Loading

Seems there is a failure
https://builds.apache.org/job/maven-box/job/maven-jar-plugin/job/MJAR-260-Automatic-Module-Name/2/

Wrong source file encoding used for javac? Locally, all tests and ITs are green.

I wonder if we should fix other plugins, namely the shade plugin

Sure. Here it is more relevant to exclude a /module-info.class from shading.

@sormuras sormuras force-pushed the MJAR-260-Automatic-Module-Name branch 2 times, most recently from 51d4303 to 16c0d21 Compare November 16, 2018 17:43
@sormuras
Copy link
Contributor Author

sormuras commented Nov 16, 2018

Seems there is a failure
https://builds.apache.org/job/maven-box/job/maven-jar-plugin/job/MJAR-260-Automatic-Module-Name/2/

Wrong source file encoding used for javac? Locally, all tests and ITs are green.

No, the real reason was the _ (underscore) which is not invalid on Java 7. Removed the check as it is not relevant in this context. Force-pushed changes to this branch.

@sormuras
Copy link
Contributor Author

sormuras commented Nov 16, 2018

Build No 4 is green (read: blue). Waiting for @khmarbaise or @rfscholte to chime in.

https://builds.apache.org/job/maven-box/job/maven-jar-plugin/job/MJAR-260-Automatic-Module-Name/

khmarbaise
khmarbaise reviewed Nov 16, 2018
View reviewed changes
@sormuras sormuras removed the request for review from rfscholte November 16, 2018 19:18
@sormuras sormuras force-pushed the MJAR-260-Automatic-Module-Name branch from 16c0d21 to 617f54c Compare November 19, 2018 17:21
@sormuras
[MJAR-260] Upgrade to Archiver 3.3.0 and add ITs
aff0671 
This PR updates the dependency to the version `3.3.0` of the Maven
Archiver and introduces two integration tests that a) verify that
JAR containing a valid name is created and b) that no JAR file is
created when it would contain an invalid automatic module name.

Addresses https://issues.apache.org/jira/browse/MJAR-260
@sormuras sormuras force-pushed the MJAR-260-Automatic-Module-Name branch from 617f54c to aff0671 Compare November 24, 2018 22:42
@sormuras sormuras merged commit b59d531 into master Dec 1, 2018
@sormuras sormuras deleted the MJAR-260-Automatic-Module-Name branch December 1, 2018 15:42
@jira-importer
Copy link

jira-importer commented Jun 9, 2025

Resolve #409

1 similar comment
@jira-importer
Copy link

jira-importer commented Jun 9, 2025

Resolve #409

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@khmarbaise khmarbaise khmarbaise left review comments

@eolivelli eolivelli eolivelli approved these changes

Assignees

@sormuras sormuras

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sormuras @eolivelli @jira-importer @khmarbaise