Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on PR #160: URL: https://github.com/apache/creadur-rat/pull/160#issuecomment-1832652351 @ottlinger absolutely :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre closed pull request #160: RAT-322: add option to be able to scan hidden directories and minor cleanups URL: https://github.com/apache/creadur-rat/pull/160 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
ottlinger commented on PR #160: URL: https://github.com/apache/creadur-rat/pull/160#issuecomment-1832623402 @jbonofre can we close this MR here as discussions are continuing in https://github.com/apache/creadur-rat/pull/166 (just to not confuse any reviewer/reader). Thanks for your contributions. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on code in PR #160: URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1404466066 ## apache-rat-core/src/main/java/org/apache/rat/walker/DirectoryWalker.java: ## Review Comment: Agree, it's what I did in the other PR. I can cancel this one in favor of the new one. Can you please take a look on the new PR ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
Claudenw commented on code in PR #160: URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1404458033 ## apache-rat-core/src/main/java/org/apache/rat/walker/DirectoryWalker.java: ## Review Comment: I think my approach would be adding the `addDirectoryFilter` to the `ReportConfiguration` so we have the groundwork for the other cases. Then add the `--scan-hidden-directories` option (or equivalent) to all the clients. (CLI, Maven, ANT). Making sure the --scan-hidden-directories is implemented across all the clients is part of RAT-323. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on code in PR #160: URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1403606229 ## apache-rat-core/src/main/java/org/apache/rat/walker/DirectoryWalker.java: ## Review Comment: I opened #166 using `DirectoryFilter`. @Claudenw @ottlinger let me know which approach you prefer :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on code in PR #160: URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1403497746 ## apache-rat-core/src/main/java/org/apache/rat/walker/DirectoryWalker.java: ## Review Comment: I experimented using `IOFileFilter` (leveraging `HiddenFileFilter`, `FalseFileFilter`, etc). Adding `addDirectoryFilter()` to `ReportConfiguration` is OK (and using `IOFileFIlter#and()` to add multiple filters. At code level, it works fine, but I have a concern about the "usage", especially on the CLI and Maven. I think for the CLI, we should keep the proposed option (`--scan-hidden-directories` for CLI for instance) to simplify the use. Thoughts ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on code in PR #160: URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1377878125 ## apache-rat-core/src/main/java/org/apache/rat/walker/DirectoryWalker.java: ## Review Comment: OK, let me update the PR then. Thanks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
Claudenw commented on code in PR #160: URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1377291820 ## apache-rat-core/src/main/java/org/apache/rat/walker/DirectoryWalker.java: ## Review Comment: If you add the AbstractFileFilter then it is probably advisable to change the internal representation of ReportConfiguration.inputFileFilter to that type as well. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
Claudenw commented on code in PR #160: URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1377289839 ## apache-rat-core/src/main/java/org/apache/rat/walker/DirectoryWalker.java: ## Review Comment: I agree. However, if we switch to org.apache.commons.io.filefilter.AbstractFileFilter we can merge multiple filters together. What we can do is modify the ReportConfiguration so that - by default set the directory filter to ignore dot prefixed directories. - if addDirectoryFilter() is called with NULL we remove the directory filter. - If addDirectoryFilter() is called with a non-null argument then the argument is "and"ed to the filter. Alternatively we could create a FilterBuilder that constructs filters using "and" and "or" methods to build an AbstractFileFilter and accept the builder as the argument for a 'setDirectoryFilter()' method. Then passing NULL could set the internal value to org.apache.commons.io.filefilter.TrueFileFilter (accepting all directories). I think that creating a builder will make specifying options in ANT UI easier. However, for now we can make due with adding a directoryFilter instance variable of type org.apache.commons.io.filefilter.AbstractFileFilter and setting it to ignore dot prefixed files by default. Then your current method can simpley change that to org.apache.commons.io.filefilter.TrueFileFilter. Later we can figure out how to specify more complex filters. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on code in PR #160:
URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1377225416
##
apache-rat-plugin/src/main/java/org/apache/rat/mp/RatCheckMojo.java:
##
@@ -109,6 +118,7 @@ public void execute() throws MojoExecutionException,
MojoFailureException {
return;
}
ReportConfiguration config = getConfiguration();
+config.setScanHiddenDirectories(scanHiddenDirectories);
Review Comment:
Ack, I do the change. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
Claudenw commented on code in PR #160:
URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1377224154
##
apache-rat-plugin/src/main/java/org/apache/rat/mp/RatCheckMojo.java:
##
@@ -109,6 +118,7 @@ public void execute() throws MojoExecutionException,
MojoFailureException {
return;
}
ReportConfiguration config = getConfiguration();
+config.setScanHiddenDirectories(scanHiddenDirectories);
Review Comment:
At line 175 in this file there is the method getConfiguration(). That
method should take all the options from the UI and create a
ReportConfiguration that represents the state of the UI options. line 121
should be moved into that method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on code in PR #160:
URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1377218838
##
apache-rat-plugin/src/main/java/org/apache/rat/mp/RatCheckMojo.java:
##
@@ -109,6 +118,7 @@ public void execute() throws MojoExecutionException,
MojoFailureException {
return;
}
ReportConfiguration config = getConfiguration();
+config.setScanHiddenDirectories(scanHiddenDirectories);
Review Comment:
Maybe I don't understand: `setScanHiddenDirectories()` is in
`getConfiguration()` right ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on code in PR #160: URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1377217935 ## apache-rat-core/src/main/java/org/apache/rat/walker/DirectoryWalker.java: ## Review Comment: I'm not sure: I think it's already covered by the excludes. If we are able to read *all* directories (including the hidden files), then we can filter with exclude. I think introducing a filter for "include" would be confusing. I would rather read all directories and delegate to exclude. A valid proposal would be to scan all directories by default (including hidden ones) and use excludes. Thoughts ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
Claudenw commented on PR #160: URL: https://github.com/apache/creadur-rat/pull/160#issuecomment-1786697563 Thank you for the DirectoryWalkerTest. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
Claudenw commented on code in PR #160:
URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1377179769
##
apache-rat-plugin/src/main/java/org/apache/rat/mp/RatCheckMojo.java:
##
@@ -109,6 +118,7 @@ public void execute() throws MojoExecutionException,
MojoFailureException {
return;
}
ReportConfiguration config = getConfiguration();
+config.setScanHiddenDirectories(scanHiddenDirectories);
Review Comment:
This line should be in the getConfiguration() method.
There is a ticket to change the way the tests are executed against UIs and
it will be necessary for the getConfiguration() to handle all configuration
changes that are driven by UI options..
##
apache-rat-core/src/main/java/org/apache/rat/walker/DirectoryWalker.java:
##
Review Comment:
I think that the DirectoryWalker should accept a second FilenameFIlter
rather than a boolean to limit the files, similar to the way the files are
filtered. This will give us more control over what files to read -- I see this
as being necessary for some of the harmonization work that is coming.
Specifically reading specific hidden directories.
This change means that the configuration needs a pair of methods to set and
get the input directory filter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on PR #160: URL: https://github.com/apache/creadur-rat/pull/160#issuecomment-1786512973 @ottlinger sure, I will :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
ottlinger commented on PR #160: URL: https://github.com/apache/creadur-rat/pull/160#issuecomment-1785002625 @jbonofre can you add an entry in the changelog? Thanks -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on PR #160: URL: https://github.com/apache/creadur-rat/pull/160#issuecomment-1784902812 I quickly discussed with @Claudenw on Slack. I think it's aligned. As I detected issue in missing ASF headers in files located in hidden directories, it would be great to release a new rat version including this improvement. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on code in PR #160:
URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1375978739
##
apache-rat-core/src/main/java/org/apache/rat/ReportConfiguration.java:
##
@@ -194,7 +195,7 @@ public void addLicense(ILicense license) {
* Adds multiple licenses to the list of licenses. Does not add the
licenses to
* the list of approved licenses.
*
- * @param license The license t oadd.
+ * @param licenses The licenses to odd.
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
ottlinger commented on code in PR #160:
URL: https://github.com/apache/creadur-rat/pull/160#discussion_r1375938723
##
apache-rat-core/src/main/java/org/apache/rat/ReportConfiguration.java:
##
@@ -194,7 +195,7 @@ public void addLicense(ILicense license) {
* Adds multiple licenses to the list of licenses. Does not add the
licenses to
* the list of approved licenses.
*
- * @param license The license t oadd.
+ * @param licenses The licenses to odd.
Review Comment:
to add
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
ottlinger commented on PR #160: URL: https://github.com/apache/creadur-rat/pull/160#issuecomment-178486 @Claudenw is this in sync with what you proposed in RAT-323? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] RAT-322: add option to be able to scan hidden directories and minor cleanups [creadur-rat]
jbonofre commented on PR #160: URL: https://github.com/apache/creadur-rat/pull/160#issuecomment-1784632099 @ottlinger As discussed in Jira -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
