Skip to content

MVN: Spotless google-java-format #114

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

Merged
merged 7 commits into from
Mar 18, 2025

Conversation

Korbik
Copy link
Contributor

@Korbik Korbik commented Feb 12, 2025

No description provided.

@ghubstan
Copy link

I'm commenting here just to say I'm following matrix and the biscuit-java PRs, waiting for your team to say when I can work on the API changes -- to consistently use camelCase for all method and variable names.

Also, this PR looks good to me. (A minimal configuration to start with is best.)
I've run the plugin's apply task on the code base many times, and it never broke an existing test.
But as you all know, running apply and trying to merge its changes will probably result in a lot of file conflicts when attempting to merge existing, open PRs, before the code base is frozen.

@Korbik Korbik force-pushed the spotless_google_format branch 3 times, most recently from dc97661 to 75677df Compare February 21, 2025 20:19
@Korbik Korbik force-pushed the spotless_google_format branch from 75677df to 517a81c Compare March 3, 2025 09:38
@ghubstan
Copy link

ghubstan commented Mar 6, 2025

I assumed your checkstyle.xml was a custom file, but it looks like you downloaded the google_checks.xml file, and renamed and commited it as google_checks.xml.

Did you get it from here?

If so, if you rename it to google_checks.xml, nobody will assume it is a custom config file.

@ghubstan
Copy link

ghubstan commented Mar 6, 2025

Regarding the checkstyle plugin configuration in the pom.xml file's <reporting> section...

Is it your intention to run the checkstyle task via the mvn site task? This may be ambiguous; you can replace the maven site configuration (delete it), and add a plugin configuration below your com.diffplug.spotless plugin's configuration, like the example below:

            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-checkstyle-plugin</artifactId>
                <version>${maven-checkstyle-plugin.version}</version>
                <configuration>
                    <configLocation>google_checks.xml</configLocation>
                    <consoleOutput>true</consoleOutput>
                    <includeTestSourceDirectory>true</includeTestSourceDirectory>
                </configuration>
                <executions>
                    <execution>
                        <goals>
                            <goal>check</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>

Instead of running mvn site to generate the checkstyle report, you would run task mvn checkstyle:checkstyle.
This would generate the html report in target/reports/checkstyle.html.

@ghubstan
Copy link

ghubstan commented Mar 7, 2025

My two previous comments do not point out errors; they are subjective opinions about file naming and configuration of different tasks that do the same thing, i.e,. mvn site vs mvn checkstyle:checkstyle.

But it looks like my assumption about you using an unmodified google_checkstyle.xml config might be wrong because you say in PR 116: Mix of sun_checks and Google Format. If so, never mind what I said about changing the name of checkstyle.xml to google_checks.xml.

And I am not qualified to comment on the Signer related commits in this PR.

So, as per Can anyone tell me what is ACK and utACK means?, I say utACK (untested ACK), because I tested the new plugin tasks, but not the Signer changes.

@Korbik
Copy link
Contributor Author

Korbik commented Mar 10, 2025

Just to clarify few things. The checkstyle.xml is indeed the google_checks.xml in the version 10.21.3.

It is mostly for the just in case of customisation and if plugins use the checkstyle.xml file to validate the code.

The mix between sun_checks and google_checks are because of misconfiguration of the plugin at the beginning of the task. It is still a good practice from my point of view but it can still be lot of work. Google Format is less strict.

I only use mvn checkstyle:checkstyle.

@Korbik Korbik merged commit 5950134 into eclipse-biscuit:main Mar 18, 2025
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants