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

Add support for re-introduced warnings in scala 3.3.0 #147

Merged
merged 3 commits into from
Jul 6, 2023

Conversation

amumurst
Copy link
Contributor

This PR prepares the plugin for the official scala 3.3.0 release (it is tagged, just not released quite yet https://github.com/lampepfl/dotty/releases/tag/3.3.0). It adds back all the warnings for unused and discarded values that is re-added in this release.

@armanbilge armanbilge linked an issue May 26, 2023 that may be closed by this pull request
@amumurst
Copy link
Contributor Author

Note that this adds the specific unused:X flags and not the unused:all. I have no strong opinion on which is better for the plugin. Adding all would mean that if another is added in 3.3.1, users of the plugin would have it automatically added when upgrading the scala version in their project.

@KamilPostrozny
Copy link

KamilPostrozny commented May 29, 2023

@amumurst personally I would go with unused:all for reasons you have mentioned. Note that for now all scala versions that support any unused option also support the rest of them. So for now it doesn't make sense - at least to me - to separate them.

@Daenyth
Copy link

Daenyth commented May 30, 2023

If we use all, how easy is it for plugin users to disable one specific lint? Say you want to disable "unused:import" for local dev for example

@froth
Copy link
Contributor

froth commented Jun 19, 2023

This looks great, I would love to use this on 3.3.0.

Is there a plan to get this released in the near future?

Copy link
Member

@armanbilge armanbilge left a comment

Choose a reason for hiding this comment

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

We've enabled these same flags in the sbt-typelevel 0.5.0 RCs and they seem to be working well. :shipit:

If a regular maintainer neither objects nor steps up I will try and get this out later this week. Sorry for the delay folks 😅

Comment on lines +208 to +209
"-source",
"3.0-migration",
Copy link
Member

Choose a reason for hiding this comment

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

sbt-typelevel was making this mistake as well. It seems this is not the sort of thing a plugin should be doing. But that will have to be fixed in another PR / breaking release.

Without the project author explicitly opting into it. I think -source is dangerous and should not be enabled without thinking through all the consequences (and ideally it should only be used temporarily during a migration)

Copy link
Member

Choose a reason for hiding this comment

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

Don't worry - this isn't actually enabled by default, it's just enabled in the test suite to make sure that users can manipulate options for each mode separately.

Copy link
Member

Choose a reason for hiding this comment

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

@armanbilge
Copy link
Member

I can merge this PR, but publishing appears to be broken 😕 if the credentials are borked which is what I suspect, I don't think I can personally fix that.

One path forward would be to start publishing under org.typelevel since we have org-wide credentials. That may require re-working the build and CI a bit: either to make our credential setup work with the existing sbt-ci-release, or switching to sbt-typelevel(-ci-release). Unfortunately I don't have time to look into this: I think we'll need someone new to step up as a regular maintainer.

@DavidGregory084
Copy link
Member

Apologies all, I'm really struggling to make time for sbt-tpolecat - life and work are conspiring against me atm. This PR looks absolutely great though, and I think we should merge it!

However, I'm not sure I can fix publishing either as I don't have access to repository level secrets in this repo anymore:
image

I think @armanbilge is right that this means publishing under org.typelevel, or alternatively, we can find a way to get the previous creds handed over to one of the org admins until the repo is in the right place to publish under org.typelevel?

On the plus side, there is an artifact migration ready to go in Scala Steward if we decide to go with the former route.

@armanbilge
Copy link
Member

@typelevel/steering would one of you be able to give @DavidGregory084 admin access to this repo so they can install updated secrets? Thanks!

@samspills
Copy link

@DavidGregory084 I've given you admin access in the repo settings!

@DavidGregory084
Copy link
Member

Thanks @samspills @armanbilge!

Publishing creds should be fixed up now: although I no longer use LastPass, I changed my Sonatype password as a precaution after the LastPass breach.

@DavidGregory084 DavidGregory084 merged commit 9fb33d4 into typelevel:main Jul 6, 2023
6 checks passed
@DavidGregory084
Copy link
Member

Thanks @amumurst!

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.

New linting options for 3.3.0
9 participants