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

Improvements to checkstyle-tester #531

Open
nrmancuso opened this issue Dec 24, 2020 · 5 comments
Open

Improvements to checkstyle-tester #531

nrmancuso opened this issue Dec 24, 2020 · 5 comments
Assignees

Comments

@nrmancuso
Copy link
Member

nrmancuso commented Dec 24, 2020

From discussion at checkstyle/checkstyle#9121 (comment):

#530 - forgo tests. Looking at https://github.com/checkstyle/checkstyle/blob/master/.ci/no-exception-test.sh#L26-L27, it doesn't look like CI cares if HTML is created or not. It just requires maven to fail the build if it fails. Launch will eventually be removed too.
#524 and #527 - finish PR for new github repos.
#523 - first part of merging launch into diff. lets make this easy and just do copy/paste of what we can. ¿Maybe look into creating groovy tests?
#529 - Complete merging.
#367 - start some removals.
#273 - complete removals.

I see it as beneficial if we could make it show how the groovy scripts will behave with your new changes... Can you recommend the best way to do this?

https://groovy-lang.org/testing.html
Testing is going to be new for groovy. This also brings up a thought brought up before. Should we keep this as groovy or is it becoming so complex we should switch it to a Java project.

This issue is simply an umbrella to maintain the order of issues to address, since the original issue where this was discussed (checkstyle/checkstyle#9121 (comment)) will be closed.

@nrmancuso
Copy link
Member Author

Minor addition, Sevntu also uses launch.groovy . See https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/.ci/wercker.sh#L35 . I am making updates to existing issues as I see/remember things we need to be mindful of in this revamp.

Originally posted by @rnveach in checkstyle/checkstyle#9121 (comment)

@rnveach
Copy link
Member

rnveach commented Dec 28, 2020

@nmancus1 Feel free to start when you are ready.

@rnveach
Copy link
Member

rnveach commented Jan 13, 2021

#524 and #527 - finish PR for new github repos.

This is optional for the improvements and can be skipped since there is an issue in merging it.

@nrmancuso
Copy link
Member Author

@rnveach if you agree with comments at #523 (comment), then I can proceed with #529 this week. I will plan on adding groovy tests in #529 in this case.

@nrmancuso
Copy link
Member Author

nrmancuso commented Feb 8, 2021

Update: I am currently working on #529. Some thoughts to leave here:

Regarding #273:
I plan to use @rnveach's scripts located at https://github.com/rnveach/contribution/tree/rvmaster/checkstyle-tester as a starting point, namely the logic from launch_diff.sh to generate reports using checkstyle, not maven-checkstyle-plugin.

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

No branches or pull requests

2 participants