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

Backport "Export diagnostics (including unused warnings) to SemanticDB" #20315

Merged
merged 3 commits into from
Jun 22, 2024

Conversation

bjaglin
Copy link

@bjaglin bjaglin commented May 1, 2024

Backports #17835 from @tanishiking to the LTS branch. Fixes (again) #17535, so that library authors stuck on LTS can benefit from it, at the very least through scalafix & metals.

  • First commit is a cherry-pick of 2fa54e8, with conflicts handled automatically
  • Second commit is an update to the expected semanticdb output, needed due to
    • the drift in the input files with the main branch
    • an update in a warning message

@bjaglin bjaglin marked this pull request as ready for review May 1, 2024 18:26
@@ -1364,6 +1364,7 @@ Occurrences:

Diagnostics:
[30:12..30:17): [warning] unused explicit parameter
[48:13..48:13): [warning] `:` after symbolic operator is deprecated; use backticks around operator instead
Copy link
Author

Choose a reason for hiding this comment

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

Comment on lines 3739 to 3741
[30:11..30:18): [warning] unset local variable, consider using an immutable val instead
[30:20..30:28): [warning] unset local variable, consider using an immutable val instead
[31:15..31:25): [warning] unset local variable, consider using an immutable val instead
Copy link
Author

Choose a reason for hiding this comment

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

Comment on lines 1759 to 1761
[24:53..24:63): [warning] An inline given alias with a function value as right-hand side can significantly increase
generated code size. You should either drop the `inline` or rewrite the given with an
explicit `apply` method.
Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Member

@tanishiking tanishiking left a comment

Choose a reason for hiding this comment

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

LGTM from my side 👍

Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

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

tests look good

@WojciechMazur WojciechMazur merged commit b26ae8e into scala:lts-3.3 Jun 22, 2024
17 checks passed
@bjaglin
Copy link
Author

bjaglin commented Aug 1, 2024

@WojciechMazur this PR does not seem to be present in the release notes of 3.3.4-RC1 (well done for all the work there!) even though it was merged. Is it maybe because it's a backport that you didn't do yourself?

@WojciechMazur
Copy link
Contributor

Sorry, I must have messed something up when categorizing the PRs. I can find this PR in git log 3.3.3..3.3.4-RC1 --oneline --merges | grep '#20315' which is the initial input to the changelog.
I've added it manually to the release notes and I'll need to remember about this one when preparing changelog for 3.3.4 final

@bjaglin
Copy link
Author

bjaglin commented Aug 4, 2024

Thanks @WojciechMazur. Don't worry for final, I will report back here if I see it missing, FWIW.

I wonder though - can there be other PRs also missing from the changelog?

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.

4 participants