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

refactor: stop shading launcher and bloopgun #1961

Merged
merged 1 commit into from
Dec 23, 2022
Merged

refactor: stop shading launcher and bloopgun #1961

merged 1 commit into from
Dec 23, 2022

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented Dec 22, 2022

Looking back I can't find a reason as to why this was done, so my
assumption is that this was done as a precaution. Looking through the
deps they are quite minimal. Testing around with Metals using the
unshaded version of launcher doesn't cause any issues. I also see that
multiple apps are using bloop-rifle instead, and that's also not shaded,
arguably has more deps, and isn't causing issues. If this is ok, we can
get rid of the entire custom shading logic we have that really
simplifies things and also allows us to no longer publish both shaded
and not shaded versions of launcher and bloopgun. If we detect problems,
we can easily then bring in sbt-shading.

There is a discussion related to this with more details in #1958.

Also note that if we want to actually slim down the deps in these, I think it's doable. For example we bring in jsoniter in bloopgun only to read the global config since it's json, and that config has two fields.

Looking back I can't find a reason as to why this was done, so my
assumption is that this was done as a precaution. Looking through the
deps they are quite minimal. Testing around with Metals using the
unshaded version of launcher doesn't cause any issues. I also see that
multiple apps are using bloop-rifle instead, and that's also not shaded,
arguably has more deps, and isn't causing issues. If this is ok, we can
get rid of the entire custom shading logic we have that really
simplifies things and also allows us to no longer publish both shaded
and not shaded versions of launcher and bloopgun. If we detect problems,
we can easily then bring in sbt-shading.
Copy link
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

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

Sorry, for some reason I did not see this PR earlier.

It's crazy good to see how much stuff you can remove from the build without breaking anything.

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

LGTM! Let's drop it for the time being and if needed we can add it back, but using an existing plugin.

@ckipp01
Copy link
Member Author

ckipp01 commented Dec 23, 2022

lol I feel like I've just been taking an 🪓 to this thing. Love it.

we can add it back, but using an existing plugin.

Exactly my thoughts as well.

@ckipp01 ckipp01 merged commit 830096f into main Dec 23, 2022
@ckipp01 ckipp01 deleted the shadingBeGone branch December 23, 2022 17:50
@ckipp01 ckipp01 mentioned this pull request Dec 24, 2022
16 tasks
Sign up for free to 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.

None yet

3 participants