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

Fix issue 21372: remove premature deprecation check. #11940

Conversation

FeepingCreature
Copy link
Contributor

@FeepingCreature FeepingCreature commented Nov 9, 2020

Far as I can tell, without that check the deprecation just triggers later on.

Not sure if this is correct, let's see what the testsuite says.

edit: Oh yeah, I need to target stable cause it's a regression.

@dlang-bot
Copy link
Contributor

dlang-bot commented Nov 9, 2020

Thanks for your pull request and interest in making D better, @FeepingCreature! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
21372 regression False deprecation raised for templated overloaded struct method

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "stable + dmd#11940"

@FeepingCreature FeepingCreature force-pushed the fix/21372-remove-premature-deprecation-check branch from ad6e5c5 to 28686dc Compare November 9, 2020 08:15
@FeepingCreature FeepingCreature changed the base branch from master to stable November 9, 2020 08:15
@FeepingCreature FeepingCreature force-pushed the fix/21372-remove-premature-deprecation-check branch from 28686dc to d2730e5 Compare November 9, 2020 08:32
@FeepingCreature
Copy link
Contributor Author

Looks like deprecation of variables broke. Okay, hm. Lemme look.

@FeepingCreature FeepingCreature force-pushed the fix/21372-remove-premature-deprecation-check branch from d2730e5 to 2497022 Compare November 9, 2020 08:51
@FeepingCreature
Copy link
Contributor Author

Re-added the check only for variable declarations.

@FeepingCreature
Copy link
Contributor Author

I think the buildkite failure is spurious. Also, what's happening with the required hanging test passes?

@thewilsonator
Copy link
Contributor

Buildkite has been fixed, try force pushing.

Keep the check only for variable declarations.
@FeepingCreature FeepingCreature force-pushed the fix/21372-remove-premature-deprecation-check branch from 2497022 to d3a058a Compare November 16, 2020 13:51
Copy link
Member

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

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

It's weird that VarDeclaration is done at this check and the rest later, and another red flag, but you got test coverage and improved the status quo, so 🤷

@Geod24 Geod24 merged commit a64d50d into dlang:stable Nov 16, 2020
@Geod24
Copy link
Member

Geod24 commented Nov 16, 2020

Slight note: You can use Fix 21372: .... now in order to keep commit messages shorter.

@FeepingCreature FeepingCreature deleted the fix/21372-remove-premature-deprecation-check branch November 16, 2020 15:26
@CyberShadow
Copy link
Member

This pull request introduced a regression:
https://issues.dlang.org/show_bug.cgi?id=23954

@FeepingCreature
Copy link
Contributor Author

FeepingCreature commented Jun 2, 2023

This pull request introduced a regression:
https://issues.dlang.org/show_bug.cgi?id=23954

This pull request fixed the regression:
#15077

:-)

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