Skip to content

only warn about Vararg wrapping once #58308

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

Closed
wants to merge 1 commit into from
Closed

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented May 2, 2025

This quiets down the long message in #38136 to be once-per-process when running with --depwarn=yes instead of spamming log files continually. We probably should have considered doing this a long time ago, but hopefully better now than never.

This quiets down the long message in #38136 to be once-per-process when
running with `--depwarn=yes` instead of spamming log files continually.
We probably should have considered doing this a long time ago, but
hopefully better now than never.
@vtjnash vtjnash added needs decision A decision on this change is needed backport 1.12 Change should be backported to release-1.12 backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 labels May 2, 2025
@KristofferC KristofferC mentioned this pull request May 5, 2025
53 tasks
@vtjnash vtjnash added the triage This should be discussed on a triage call label May 7, 2025
@oscardssmith
Copy link
Member

Triage thinks doing this without without printing file+line-number is a really bad idea since it will make it impossible to tell whether you've fixed an instance or not.

In general, triage thinks that we really need to report line numbers from our depwanrs.

@oscardssmith oscardssmith removed the triage This should be discussed on a triage call label May 8, 2025
@vtjnash
Copy link
Member Author

vtjnash commented May 8, 2025

If you still get the message, then you haven't fixed it? I don't quite follow that concern.

@oscardssmith
Copy link
Member

If you have 3 instances of the message, and you de-duplicate the warnings, when you fix one of the 3 places causing a warning, there still exist 2 more, so you will not see any change in output.

@KristofferC KristofferC mentioned this pull request May 9, 2025
58 tasks
@vtjnash vtjnash closed this May 10, 2025
@vtjnash
Copy link
Member Author

vtjnash commented May 10, 2025

Alright, spam the user with useless message it is

@oscardssmith
Copy link
Member

Can we get a different PR that adds the line numbers to make the message useful instead?

@KristofferC KristofferC removed backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 labels Jul 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs decision A decision on this change is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants