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.

Sign up for GitHub

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

Jump to bottom

Bump spotless and versionlock #2829

Closed
wants to merge 4 commits into from
Closed

Bump spotless and versionlock #2829

wants to merge 4 commits into from

Conversation

daberkow
Copy link

@daberkow daberkow commented Jul 11, 2024 •
edited
Loading

Before this PR

Issues arose running spotless checks, as shown in #2828

After this PR

==COMMIT_MSG==
Update spotless java to 6.25.0 from 6.6.0
==COMMIT_MSG==

Possible downsides?

It was noted that updates to spotless "Newer spotless versions have issues resolving dependencies at configuration time", but the build seems to be working.

@changelog-app
Copy link

changelog-app bot commented Jul 11, 2024 •
edited by daberkow
Loading

Generate changelog in changelog/@unreleased

What do the change types mean?
  • feature: A new feature of the service.
  • improvement: An incremental improvement in the functionality or operation of the service.
  • fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
  • break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
    and external consumers of the service's API (e.g. customer-written software or integrations).
  • deprecation: Advertises the intention to remove service functionality without any change to the
    operation of the service itself.
  • manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
    performing database surgery, ...) at the time of upgrade for it to succeed.
  • migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?
  • ❗The break and manual task changelog types will result in a major release!
  • 🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
  • ✨ All others will result in a minor version release.

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Update spotless java to 6.24.0 from 6.6.0

Check the box to generate changelog(s)

  • Generate changelog entry

@daberkow
Copy link
Author

This has been tested locally against a repo showing the issue, and bumping did correct the issue. I realized 6.25.0 is out, and will bump again.

@daberkow
Copy link
Author

I am doing some checks, 6.25.0 is passing in circle but failing on my local system. 6.24.0 is passing in both. Trying to figure out why.

pkoenig10
pkoenig10 reviewed Jul 11, 2024
View reviewed changes
versions.props
Comment on lines -48 to -49
# Newer spotless versions have issues resolving dependencies at configuration time
com.diffplug.spotless:spotless-plugin-gradle = 6.6.0
Copy link
Member

Choose a reason for hiding this comment

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

The issues that prompted this pin in #2312 is still open:

diffplug/spotless#1380

Do we have reason to believe this issue is fixed?

Copy link
Author

Choose a reason for hiding this comment

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

In testing this and coil mentioned in the issue. Coil still has it, this repo seems to currently not have it, and people seem to think its related to indirect dependencies being loaded. Gradle does correct the issue if it comes up, it just causes a perf hit. Without bumping spotless, repos using OpenJFx and some other libs can compile at all. If we do not update this lib, repos with that issue will be forced to disable baseline-spotless entirely.

Copy link
Author

Choose a reason for hiding this comment

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

The bug reports say that gradle has a catch for this and the worst case scenario is: if there are indirect dependencies and it has to work through them, hitting this regression, then it is a small perf hit, but doesnt fail builds. Where as without this we cant compile UI apps at all. :/

Copy link
Author

Choose a reason for hiding this comment

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

Is there a path to get spotless updated here?

@daberkow
Copy link
Author

#2313
diffplug/spotless#1380
With gradle-consistent-versions this will cause a full build failure.

Pointed out, for anyone who has issues, you can add the newer spotless version to your classpath to override what is given here, and work from there.

@daberkow daberkow closed this Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkoenig10 pkoenig10 pkoenig10 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@daberkow @pkoenig10 @svc-changelog