search

sevntu-checkstyle / sevntu.checkstyle

Additional Checkstyle checks, that could be added as extension to EclipseCS plugin and maven-checkstyle-plugin, Sonar checkstyle plugin, extension for CheckStyle IDEA plugin.
http://sevntu-checkstyle.github.io/sevntu.checkstyle/
190 stars 147 forks source link

Discuss removing suppression for settersHaveSinceTag #1022

Open stoyanK7 opened 1 year ago

stoyanK7 commented 1 year ago

In https://github.com/checkstyle/checkstyle/pull/13455 we add a MatchXPath check to ensure that all property setters have a @since property in their Javadoc. For example

/**
...
@since 1.1
*/
public final void setSomething() ..

In https://github.com/sevntu-checkstyle/sevntu.checkstyle/pull/1023 we add a suppression for all checks in this repository because the CI in checkstyle repo is failing https://checkstyle.semaphoreci.com/jobs/b80149c4-f163-4908-a4ab-98a19bcb625e

ERROR] [checkstyle] [ERROR] /home/semaphore/checkstyle/.ci-temp/sevntu.checkstyle/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/annotation/ForbidAnnotationCheck.java:66:5: All property setters of a module should contain a @since tag. [[ERROR] /home/semaphore/checkstyle/.ci-temp/sevntu.checkstyle/sevntu-checks/config/ant-phase-verify.xml:116: Checkstyle failed: Got 115 errors and 0 warnings.] 01:07
[ERROR] [checkstyle] [ERROR] /home/semaphore/checkstyle/.ci-temp/sevntu.checkstyle/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/annotation/ForbidAnnotationCheck.java:79:5: All property setters of a module should contain a @since tag. [settersHaveSinceTag]
...
[ERROR] /home/semaphore/checkstyle/.ci-temp/sevntu.checkstyle/sevntu-checks/config/ant-phase-verify.xml:116: Checkstyle failed: Got 115 errors and 0 warnings.

I am not sure if we want to add those @sinces in this project or if it's possible. The purpose of this issue is to add an until ... on the line above the suppression of this check and discuss potentially removing it in the future

nrmancuso commented 1 year ago

I think it is reasonable to add such tags to property setters anywhere really, let's see what others think.

rnveach commented 1 year ago

I don't believe this repo has those things documented, so it will be regression hunting to identify all the versions to add. The custom branch I used referenced in the main repo's issue should be able to be used for this.

I am fine with this being done.

romani commented 1 year ago

This repo don't need this metadata for now, if we have time in future we will do this