search

apache / streampipes

Apache StreamPipes - A self-service (Industrial) IoT toolbox to enable non-technical users to connect, analyze and explore IoT data streams.
https://streampipes.apache.org
Apache License 2.0
560 stars 172 forks source link

Enable check style for the project #820

Closed RobertIndie closed 1 year ago

RobertIndie commented 1 year ago

Body

This is the issue to track the progress of check-style enablement for all modules. If you want to take over some of the modules, please feel free to leave a comment here. And thanks for all your contribution! :smiley:

Only the first level of sub-modules are listed here. If you feel that some of the modules(eg. streampipes-extensions) are heavy, please continue to split them into smaller tasks in this issue.

Note: The above todo tasks are generated by the command mvn --also-make dependency:tree | grep maven-dependency-plugin | awk '{ print "- [ ] "$(NF-1) }'. Please tell me if there are any problems.

StreamPipes Committer

tenthe commented 1 year ago

Hey @RobertIndie, I'll start with the module streampipes-rest. How can I link the subtask (TODO) to pull requests?

RobertIndie commented 1 year ago

Hi @tenthe

How can I link the subtask (TODO) to pull requests?

You can edit my Issue description and paste your PR link to the sub-task. And as you are doing now, you can link to this issue in your PR description.

tenthe commented 1 year ago

Hi all, I just enabled checkstyle in the module streampipes-processors-geo-jvm. There we have the case that some files have a different license (MIT). Further, there are other checkstyle violations (e.g. an author in comments) that we can't remove. How can we deal with such cases?

Examples:

The changes are in PR #850

tenthe commented 1 year ago

@RobertIndie, currenlty I refactor the streampipes-extensions module. I just found out that the command mvn validate no longer returns an error when there is a checkstyle problem. (e.g. mvn validate -f streampipes-extensions/streampipes-processors-text-mining-flink) Could this be related to the changes in #851 or is there something I am doing wrong?

Do I need to configure anything in the module streampipes-extensions? I only added this to all submodules:

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-checkstyle-plugin</artifactId>
</plugin>
RobertIndie commented 1 year ago

Hi @tenthe Thanks for your comment.

I just found out that the command mvn validate no longer returns an error when there is a checkstyle problem.

I have created a PR to fix it: https://github.com/apache/streampipes/pull/853

RobertIndie commented 1 year ago

Hi, @tenthe

There we have the case that some files have a different license (MIT). Further, there are other checkstyle violations (e.g. an author in comments) that we can't remove. How can we deal with such cases?

How about suppressing these files here: https://github.com/apache/streampipes/blob/dev/tools/maven/suppressions.xml I also found a way to apply different rules to different files: https://stackoverflow.com/questions/25894431/checkstyle-different-rules-for-different-files . But it may introduce some complexity.

bossenti commented 1 year ago

Personally, I would prefer option 1 and only suppress the affected rules working with as less wildcards as possible. The second option can get easily confusing, as fas as I can imagine

tenthe commented 1 year ago

Ok, thans to the both of you. I used the first option and added it to PR #850. This PR should (hopefully) contain all checkstyle fixes for the streampipes-extension module