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

Automatically wire up the --enable-preview option if necessary #1033

Merged
merged 4 commits into from
Nov 18, 2020

Conversation

iamdanfox
Copy link
Contributor

Before this PR

In baseline 3.52.0 we added this new plugin as a one-liner to add the --enable-preview flag wherever necessary (palantir/gradle-baseline#1549). We're using the extraProperties thing to avoid taking a compile dependency on baseline.

After this PR

==COMMIT_MSG==
sls-packaging will automatically include the --enable-preview jvm arg in launcher-static.yml if com.palantir.baseline-enable-preview-flag is applied.
==COMMIT_MSG==

Possible downsides?

@iamdanfox iamdanfox requested a review from ferozco November 17, 2020 21:03
@changelog-app
Copy link

changelog-app bot commented Nov 17, 2020

Generate changelog in changelog/@unreleased

Type

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

Description

sls-packaging will automatically include the --enable-preview jvm arg in launcher-static.yml if com.palantir.baseline-enable-preview-flag is applied.

Check the box to generate changelog(s)

  • Generate changelog entry

Copy link
Contributor

@ferozco ferozco left a comment

Choose a reason for hiding this comment

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

What do you think about keeping all of this inside of baseline? I think it might be a bit cleaner since consolidates the preview logic in one place and stays true to the nature of baseline configuring other plugins correctly

@iamdanfox
Copy link
Contributor Author

iamdanfox commented Nov 17, 2020

Right, that's the other options for where to put this stuff. I wanted to preserve the situation we have now where there is no compilation dependency from sls-packaging to baseline or vice versa. It seems that if we put the logic in baseline and make it 'reach out' to configure sls-packaging then we'll need to have a compile dependency in order to get hold of the JavaServiceDistributionExtension.

I figured doing it this way round (with the Provider<Boolean>) avoid this kinda nicely? Also I figured that in theory there might be lots of different plugins that could all need to read this flag (e.g. maybe some gradle-graal things too??) so exposing a nice readable Provider from baseline seemed sort-of future-proof.

@iamdanfox
Copy link
Contributor Author

iamdanfox commented Nov 17, 2020

Somewhat related thought - what do you think about baking something into our manifest or something so that we can do a papaya analysis and keep track of all the releases that enable this flag?? Feels like at some point we're going to want to keep an eye on who's using this!

EDIT nvm there's a dataset in papaya of launcher-static files already, so we can just detect the --enable-preview flag in that: ri.foundry.main.dataset.e3c4b54c-99eb-48cf-9f6d-df35d58e77bf

@bulldozer-bot bulldozer-bot bot merged commit 730a9ad into develop Nov 18, 2020
@bulldozer-bot bulldozer-bot bot deleted the dfox/enable-preview branch November 18, 2020 15:18
@svc-autorelease
Copy link
Collaborator

Released 4.22.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants