Skip to content

Conversation

@tisonkun
Copy link
Member

@tisonkuntisonkun commented May 18, 2023

The first commit is the real plugin and CI change. The second commit is generated by running ./mvnw spotless:apply.

The discussion thread can be found at https://lists.apache.org/thread/12rwy3y1zcs3pjh9q30851mldgdh4sgg

If we agree on the direction, I'll file a ticket for this.

@kezhuwkezhuw self-requested a review May 20, 2023 14:42
@tisonkuntisonkun changed the title CURATOR-XXX. Apply code style with spotlessCURATOR-674. Apply code style with spotlessJun 1, 2023
@tisonkun
Copy link
MemberAuthor

Updated. PTAL @kezhuw@cammckenzie@eolivelli

Copy link
Member

@kezhuwkezhuw left a comment

Choose a reason for hiding this comment

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

+1

I reviewed the first commit and trust CI for remains.

<configuration>
<java>
<palantirJavaFormat>
<version>2.30.0</version>
Copy link
Member

Choose a reason for hiding this comment

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

Add to "properties" tag ?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Make sense. As this PR already touch a large range of code. I tend to postpone this suggestion after we merge this one.

@kezhuw
Copy link
Member

I replayed this.

  1. git fetch origin
  2. gh pr checkout -f 463
  3. git reset --hard d8c8bc3
  4. git rebase origin/master
  5. ./mvnw spotless:apply
  6. git diff 236d49f

The final step gives no output and echo $? outputs 0. So I am confident about the remains.

Look forward for this.

@tisonkun
Copy link
MemberAuthor

As we agree on the mailing list already, I'll merge this patch in hours.

@tisonkun
Copy link
MemberAuthor

Merging...

@tisonkuntisonkun merged commit 484ede9 into apache:masterJun 2, 2023
@tisonkuntisonkun deleted the spotless branch June 2, 2023 08:11
Sign up for freeto 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

@tisonkun@kezhuw