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

RAT-346: warn on duplicate license family #195

Merged

Conversation

Claudenw
Copy link
Contributor

Added code to warn on duplicate license or license family.

Also upgraded to Junit 5 as it was needed for the testing.

@Claudenw Claudenw marked this pull request as draft January 13, 2024 16:19
@Claudenw Claudenw force-pushed the RAT-346_warn_on_duplicate_license_family branch from 61846f2 to d01009d Compare January 13, 2024 16:50
@Claudenw Claudenw marked this pull request as ready for review January 13, 2024 17:53
@ottlinger ottlinger changed the title Rat-346 warn on duplicate license family RAT-346: warn on duplicate license family Jan 13, 2024
@ottlinger ottlinger self-assigned this Jan 13, 2024
* able to be opened and closed multiple times.
* Sets the style sheet for custom processing. The IOSupplier may be called
* multiple times, so the input stream must be able to be opened and closed
* multiple times.
Copy link
Contributor

Choose a reason for hiding this comment

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

btw: I guess that this new behaviour is the cause of https://issues.apache.org/jira/browse/RAT-344 ;)

@ottlinger
Copy link
Contributor

@Claudenw not sure why you removed the API module ..... if I understood it correctly we want to harmonize things among all the available RAT-options (e.g. Ant,Maven,CLI). I'd assume an API module to be handy in that regard.

@ottlinger
Copy link
Contributor

@jbonofre just fyi - a great deal of changes will come in with the jUnit5 update.

@jbonofre
Copy link
Member

@ottlinger thanks for the heads up. I will be back on rat work tomorrow.

@Claudenw
Copy link
Contributor Author

@ottlinger thx for the quick review

I'll make the change to the @tempdir annotation -- I think there are 2 files where it is needed.

I removed the API module because it consisted of a) class files that were not being used in the code but that were misleading in name; and more importantly b) because there were a number of test cases that would have been converted to Junit5 only to be removed again later.

The harmonization is not necessarily the use of a single API module but rather that the options in each of the UIs should be similarly constructed (same basic phrasing) not have 2 UIs where one says "--enable-default-licenses" (default off) and the other says "--disable-default-licenses" (default on). Also there are options that have been added to one UI that are general in nature (i.e. not part of managing the specific platform the UI is on) and are not available in the other UIs.

I will remove the commented out code. I should have before I submitted this pull request, but skipped it in favor of getting this code change infront of more eyes quickly.

I also have to make a pass to update javadocs.

@jbonofre Most of the changes in this request are the migration to Junit 5. The significant changes are the creation of the classes in the o.a.r..utils directory, the addition of methods to set the associated properties in ReportConfiguration along with the tests for those classes and changes.

The request was to warn on duplication so the solution submitted does the following:

  • Creates a ReportingSet class that is a SortedSet implementation (wrapping and delegating to a SortedSet). it intercepts the calls to add and addAll to determine if the new elements are already in the set and take an action.

    • If logging is enabled (not set to None on the very thing logging wrapper Log) write a message about the collision to the log at the specific level.
    • If the internal action (ReportingSet.Option) is set to IGNORE the change is ignored, OVERWRITE the old entry is replaced, FAIL an exception is thrown.

Finally the ReportingSet has an instance variable called duplicateFmt that is a Function<T,String> that produces the information to display in the message.

The change to ReportConfiguration are to use ReportingSet to contain the Licenses and LicenseFamilies, and supporting methods to configure (as noted above).

@Claudenw
Copy link
Contributor Author

@ottlinger Thanks for taking the time to clean this up. It looks good to me.

@ottlinger
Copy link
Contributor

@Claudenw pls give a ping if you have any local changes that need to be pushed. I`d love to merge this PR in order to continue analysing the problems on #192

@ottlinger
Copy link
Contributor

Thanks @Claudenw for the simplifications and the migration efforts!

@ottlinger ottlinger merged commit 1cdd21f into apache:master Jan 14, 2024
16 checks passed
@Claudenw Claudenw deleted the RAT-346_warn_on_duplicate_license_family branch April 23, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants