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

Update checkstyle to 10.0 and fix various related issues #8073

Merged
merged 2 commits into from
Mar 19, 2022

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Mar 18, 2022

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Update checkstyle from 9.3 to 10.0 (changelog).
  • Put checkstyle files into checkstyle/ subfolder so that the gradle task does not implicitly depend on the whole project, fixing many warnings during build and possibly increasing build performance.
    • The warnings were like this: Execution optimizations have been disabled for task ':app:runCheckstyle' to ensure correctness due to the following reasons: Gradle detected a problem with the following location: 'XXX/NewPipe'. Reason: Task ':app:runCheckstyle' uses this output of task ':app:XXX' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.4/userguide/validation_problems.html#implicit_dependency for more details about this problem.
  • Remove unused SuppressionXpathFilter from config file.
  • Remove outdated suppressions from suppressions file.
Spoiler alert

I'm adding checkstyle to NewPipeExtractor ;-)

Due diligence

- Put checkstyle files into checkstyle/ subfolder so that the gradle task does not implicitly depend on the whole project, fixing many warnings during build and possibly increasing build performance.
- Remove unused SuppressionXpathFilter from config file.
- Remove outdated suppressions from suppressions file.
@triallax triallax added the codequality Improvements to the codebase to improve the code quality label Mar 18, 2022
Copy link
Member

@litetex litetex left a comment

Choose a reason for hiding this comment

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

LGTM

It's better to use @SuppressWarnings instead of the suppressions file, so that the warning suppression is at the place where it acts.
@Stypox
Copy link
Member Author

Stypox commented Mar 18, 2022

I pushed a commit that replaces some suppressions in the suppressions file with @SuppressWarnings, so that the suppression is written right where it acts. I also added @SuppressWarnings to the four [WARN] that have been haunting us for years, so now there is no misleading red output anymore in the build process ;-)

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@triallax
Copy link
Contributor

I pushed a commit that replaces some suppressions in the suppressions file with @SuppressWarnings, so that the suppression is written right where it acts.

You know, I've always found it weird that we had checkstyle suppressions in a separate file; what if they become outdated or something? This is perfect and easier to follow.

@litetex litetex merged commit 70d9a77 into TeamNewPipe:dev Mar 19, 2022
@Stypox Stypox mentioned this pull request Apr 16, 2022
12 tasks
@Stypox Stypox deleted the bump-checkstyle branch August 4, 2022 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codequality Improvements to the codebase to improve the code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants