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

Use upstream Spotless configuration #684

Merged
merged 2 commits into from
Jun 1, 2023
Merged

Conversation

basil
Copy link
Member

@basil basil commented Apr 12, 2023

Now that the upstream plugin parent POM has a formatting profile, I am trying to adapt as many consumers to use it as possible. This repository was an early attempt at automatic formatting that used a different configuration. I do not think anyone is particularly attached to it, and maintenance would be easier moving to our project-wide standard. @jglick Any interest in merging this? I would not be upset if this PR is rejected, but if this repository is to continue using Spotless I think it would benefit from using a more standard configuration. If accepted I will add a .git-blame-ignore-revs file after this is merged.

@jglick
Copy link
Member

jglick commented Apr 12, 2023

More legible (IMO) than #392. I do not believe @bitwiseman remains active in Jenkins. I will ask around to see if anyone claims to maintain this plugin.

Copy link
Member

@jtnord jtnord left a comment

Choose a reason for hiding this comment

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

introduces a regression

mvn spotless:check passes on master but fails with this change due to presumably diffplug/spotless#1559

@basil
Copy link
Member Author

basil commented Apr 12, 2023

@bitwiseman
Copy link
Contributor

@basil @jglick
No I'm not active in Jenkins at this time. FWIW, my original goal for using spotless was to not have any more discussions or PR comments regarding formatting. This change seem to be aligned with that goal.

@jglick jglick added the chore label Jun 1, 2023
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Consistent with other repos.

@jglick
Copy link
Member

jglick commented Jun 1, 2023

(@rsandell @car-roll FYI, since you both have active PRs.)

I will merge this and follow up with an addition to .git-blame-ignore-revs as in #528.

@jglick jglick enabled auto-merge (squash) June 1, 2023 16:56
@basil
Copy link
Member Author

basil commented Jun 1, 2023

I can help with merging this change into existing branches if needed.

jglick added a commit to jglick/github-branch-source-plugin-1 that referenced this pull request Sep 8, 2023
jglick added a commit to jglick/github-branch-source-plugin-1 that referenced this pull request Sep 8, 2023
@jglick
Copy link
Member

jglick commented Sep 8, 2023

Rough merge instructions FTR

git merge 51545c3^
git stash apply
mvn spotless:apply
git checkout HEAD -- pom.xml
gh pr diff --name-only | xargs git add
git commit -m 'Spotless like in #684'
git reset --hard HEAD
git merge -Xours 51545c3
git pull origin master:master

with the stash

diff --git pom.xml pom.xml
index f1b8ba8..58b01c0 100644
--- pom.xml
+++ pom.xml
@@ -4,7 +4,7 @@
     <parent>
         <groupId>org.jenkins-ci.plugins</groupId>
         <artifactId>plugin</artifactId>
-        <version>4.16</version>
+        <version>4.61</version>
         <relativePath />
     </parent>
     <artifactId>github-branch-source</artifactId>
@@ -135,63 +135,6 @@
             </dependency>
         </dependencies>
     </dependencyManagement>
-    <build>
-        <plugins>
-            <plugin>
-                <groupId>com.diffplug.spotless</groupId>
-                <artifactId>spotless-maven-plugin</artifactId>
-                <version>2.8.1</version>
-                <executions>
-                    <execution>
-                        <id>spotless-check</id>
-                        <!-- runs in verify phase by default -->
-                        <goals>
-                            <!-- can be disabled using -Dspotless.check.skip=true -->
-                            <goal>check</goal>
-                        </goals>
-                    </execution>
-                </executions>
-                <configuration>
-                    <java>
-                        <googleJavaFormat>
-                            <style>GOOGLE</style>
-                        </googleJavaFormat>
-                    </java>
-                </configuration>
-            </plugin>
-        </plugins>
-    </build>
-    <profiles>
-        <profile>
-            <id>ci-non-windows</id>
-            <activation>
-                <property>
-                    <name>set.changelist</name>
-                </property>
-                <os>
-                    <family>!windows</family>
-                </os>
-            </activation>
-            <build>
-                <plugins>
-                    <plugin>
-                        <groupId>com.diffplug.spotless</groupId>
-                        <artifactId>spotless-maven-plugin</artifactId>
-                        <executions>
-                            <execution>
-                                <id>spotless-check</id>
-                                <!-- In CI, run check early in the build -->
-                                <phase>process-sources</phase>
-                                <goals>
-                                    <goal>check</goal>
-                                </goals>
-                            </execution>
-                        </executions>
-                    </plugin>
-                </plugins>
-            </build>
-        </profile>
-    </profiles>
     <repositories>
         <repository>
             <id>repo.jenkins-ci.org</id>

jglick added a commit to jglick/github-branch-source-plugin-1 that referenced this pull request Sep 8, 2023
@basil basil deleted the spotless branch September 8, 2023 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants