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

isStandardLicenseWithinText - match spaces after var rule #249

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

goneall
Copy link
Member

@goneall goneall commented Sep 7, 2024

Fixes #241
Fixes #234

Include code provided by @pmonk in #236 modified for version 2.0+

Thanks @pmonks and @sdheh for your patience and analysis / unit test help!

Fixes #241
Fixes #234

Include code provided by @pmonk in #236 modified for version 2.0+

Signed-off-by: Gary O'Neall <[email protected]>
@pmonks
Copy link
Collaborator

pmonks commented Sep 7, 2024

@goneall it appears that the new unit tests that leverage the CompareConsistencyHelper test class didn't make it over yet. Am I just jumping the gun, or was that an oversight?

FWIW here are the (minimal) unit tests in the original PR, though arguably more could/should be added (especially to cover the cases @sdheh identified in issue #245).

@goneall
Copy link
Member Author

goneall commented Sep 9, 2024

@goneall it appears that the new unit tests that leverage the CompareConsistencyHelper test class didn't make it over yet. Am I just jumping the gun, or was that an oversight?

FWIW here are the (minimal) unit tests in the original PR, though arguably more could/should be added (especially to cover the cases @sdheh identified in issue #245).

Thanks for the catch @pmonks - it must have gotten dropped when I rebased / merged it to the 2.0 latest.

I'll check to make sure the unit tests pass and add the tests back in in a subsequent commit.

Signed-off-by: Gary O'Neall <[email protected]>
@goneall
Copy link
Member Author

goneall commented Sep 9, 2024

@pmonks - the unit tests are now included - it passes with the SPDX_JAVA_LIB_RUN_SLOW_TESTS environment variable set to true.

@goneall
Copy link
Member Author

goneall commented Sep 18, 2024

@pmonks - Could you take another look and let me know if this is good to merge?

There's also a couple other PR's if you have time for a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants