sebawagner commented on a change in pull request #69:
URL: https://github.com/apache/openmeetings/pull/69#discussion_r415515187
##########
File path: pom.xml
##########
@@ -1084,6 +1122,22 @@
</reportSet>
</reportSets>
</plugin>
+ <!-- plugin>
Review comment:
I think delete this as its commented out and the merge.
I don't mind the reporting side. But obviously having some warnings would be
useful as overall report.
This could be also a good point to give to "novice" developer that want to
join project: Fix code style warnings :)
##########
File path: pom.xml
##########
@@ -1084,6 +1122,22 @@
</reportSet>
</reportSets>
</plugin>
+ <!-- plugin>
Review comment:
The reporting section I'm not sure if those inline configs work.
The other thing about shared config file rather than inline configs is that
the checkstyle.xml can be used in your favourite editor. So you get those
warnings while coding. Not while running maven.
##########
File path: pom.xml
##########
@@ -804,6 +805,43 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
</plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-checkstyle-plugin</artifactId>
+
<version>${maven-checkstyle-plugin.version}</version>
+ <configuration>
+ <checkstyleRules>
+ <module name = "Checker">
+ <property
name="fileExtensions" value="java,js,css,xml"/>
Review comment:
I think you enabled the file extensions successfully.
But TreeWalker only works for Java file types.
https://checkstyle.sourceforge.io/config.html#TreeWalker
> FileSetCheck TreeWalker checks individual **Java source** files and
defines properties that are applicable to checking such files.
I think js you can forget, but the google reference checkstyle config checks
additionally XML and properties files.
https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml
And those rules outside of TreeWalker in above google XML should also work
for non Java files. Except they have additional rules.
But yeah its really Java centric. For JS I think you should have a linter
filer and invoke a eslint. You can just run Eslint from command line. Or invoke
it via a maven wrapper that invokes a command line.
And it will be much better (and more easy) to re-use eslint files from other
projects for JavaScript checks. And prob also more what people expect that code
Javascript.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
