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

Ignore packages #12

Closed
wants to merge 2 commits into from
Closed

Conversation

jponge
Copy link
Contributor

@jponge jponge commented Sep 22, 2014

This feature allows ignoring some package prefixes.

This is useful when dealing with generated code.

Julien Ponge added 2 commits September 22, 2014 22:50
This allows the configuration of packages to ignore. This is useful in situations where some code is
being generated by another tool or plugin, and the generated code contains violations. A good
example is the code generated by JavaCC.

Signed-off-by: Julien Ponge <[email protected]>
if (path.endsWith(".class")) {
if (ignorePackages != null) {
for (String ignoredPrefix : ignorePackages) {
if (path.replace(File.separatorChar, '.').contains(ignoredPrefix)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of filtering package names in the Mojo, perhaps Modernizer.check should do this? One approach would be to use ClassReader.getClassName.

@gaul
Copy link
Owner

gaul commented Sep 22, 2014

@jponge This seems like a good feature although can you investigate my proposed alternative implementation? Also can you address the Checkstyle violations? I will configure CloudBees for this repository to automate this in the future. Thanks!

@jponge
Copy link
Contributor Author

jponge commented Sep 25, 2014

Haven't much time, but I'll try to get back to that PR one of these days 😄

@gaul
Copy link
Owner

gaul commented Sep 25, 2014

Adding this functionality to Modernizer.check will also allow adding a unit test.

@gaul gaul force-pushed the master branch 2 times, most recently from 4817d32 to 5c7190b Compare December 15, 2014 11:57
@gaul
Copy link
Owner

gaul commented Dec 15, 2014

@jponge I reworked your pull request as 95f86a6 and will run another release soon. Thank you for raising this issue!

@gaul gaul closed this Dec 15, 2014
@jponge
Copy link
Contributor Author

jponge commented Dec 15, 2014

Thanks Andrew. Sorry I didn't have the time to work further on it.

On Mon, Dec 15, 2014, at 13:19, Andrew Gaul wrote:

@jponge I reworked your pull request as
95f86a6 and will run another release
soon. Thank you for raising this issue!


Reply to this email directly or view it on GitHub:
#12 (comment)

@ArloL
Copy link
Contributor

ArloL commented Dec 16, 2014

This feature is really cool. Any idea when you will release?

@gaul
Copy link
Owner

gaul commented Dec 16, 2014

@ArloL I will run another release this week including this fix.

@gaul
Copy link
Owner

gaul commented Dec 17, 2014

@ArloL Released 1.2.0 to Maven Central.

@ArloL
Copy link
Contributor

ArloL commented Dec 17, 2014

Cool, thanks!

@gaul
Copy link
Owner

gaul commented Dec 17, 2014

Note that this commit introduced a regression #17.

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