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

Goethe re-bootstraps itself for jdk16+ support #53

Merged
merged 4 commits into from
Oct 29, 2021
Merged

Goethe re-bootstraps itself for jdk16+ support #53

merged 4 commits into from
Oct 29, 2021

Conversation

carterkozak
Copy link
Contributor

No description provided.

@changelog-app
Copy link

changelog-app bot commented Oct 28, 2021 •
edited by carterkozak
Loading

Generate changelog in changelog/@unreleased

Type

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

Description

Goethe re-bootstraps itself for jdk16+ support

Check the box to generate changelog(s)

  • Generate changelog entry

@carterkozak carterkozak changed the title proof-of-concept: ideally we'd leverage https://github.com/palantir/palantir-java-format/pull/562 Goethe re-bootstraps itself for jdk16+ support Oct 28, 2021
@carterkozak carterkozak requested a review from fawind October 28, 2021 18:47
@carterkozak
Copy link
Contributor Author

Tests pass on jdk17, although I haven't wired through java 17 test coverage to this project.

@carterkozak carterkozak marked this pull request as ready for review October 28, 2021 18:51
@policy-bot policy-bot bot requested a review from CRogers October 28, 2021 18:52
fawind
fawind reviewed Oct 29, 2021
View reviewed changes
goethe/src/main/java/com/palantir/goethe/BootstrappingFormatterFacade.java
.addAll(EXPORTS)
.add( // Classpath
"-cp",
System.getProperty("java.class.path"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Just flagging that this might fail on windows as this might exceed the maximum command size. But not sure if its worth investing in a workaround.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can fix it once it comes up -- we should be able to build a tiny classpath for this, but there are edge cases that I'd prefer to punt thinking about until we need to :-)

@carterkozak
Copy link
Contributor Author

I verified with a snapshot that dialogue builds are successful on java 17 with this change.

fawind
fawind approved these changes Oct 29, 2021
View reviewed changes
Copy link
Contributor

@fawind fawind left a comment

Choose a reason for hiding this comment

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

Looks good! I like using your own Main instead of dealing with all the formatter CLI options!

@bulldozer-bot bulldozer-bot bot merged commit 04f7cb6 into develop Oct 29, 2021
@bulldozer-bot bulldozer-bot bot deleted the ckozak/silly_bootstrapping branch October 29, 2021 12:31
@svc-autorelease
Copy link
Collaborator

Released 0.3.0

@carterkozak carterkozak mentioned this pull request Oct 29, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fawind fawind fawind approved these changes

@CRogers CRogers Awaiting requested review from CRogers

Assignees
No one assigned
Labels
autorelease merge when ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@carterkozak @svc-autorelease @fawind @svc-changelog