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 17340 - isNumeric!bool should not be true #5343
Conversation
Regression fixes should go onto stable so they get included in the next point release. Please use the Github UI (the button next to the PR title) to change the base branch and then manually rebase the changes onto the right commit. |
Done...
What is "the right commit"? How do I do this? |
@tsbockman I generally have to look up the directions when doing this: https://wiki.dlang.org/Starting_as_a_Contributor#Stable_Branch |
The changes seem quite simple, maybe just checkout the stable branch and recreate it might take less time. |
2ae83e3
to
83f757c
Compare
|
@JackStouffer @schveiguy Thanks. I think I've got the branch sorted out now. |
ping @wilzbach dlang bot seems to have erred here. I think when the branch was targeting stable, but included all the fixes from master, it confused the hell out of the bot and it put some unrelated bug links. But it hasn't put the correct link here (fix 17340). I'm concerned about merging this might post to wrong bug report. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hm... auto tester I think doesn't like that the PR moved from master to stable (https://auto-tester.puremagic.com/pull-history.ghtml?projectid=1&repoid=3&pullid=5343). Something's not right. @braddr? |
@schveiguy The bug report the bot says it will close is already closed, so it should be fine. Also, I had the same issue with another PR that I switched to stable. After I added the auto merge tag and the auto tester prioritized it, it was tested eventually. |
@JackStouffer What's odd is that the auto tester is testing against master. And they are new runs, so it hasn't picked up that it needs to test against stable since the retargeting occurred. I don't know if it will switch automatically. |
Autotester seems completely confused. I'm gonna try closing and reopening this PR to see if that helps. Apologies in advance if that screws things up. |
Auto-merge toggled on |
OK, that seems to have kicked some sense into the autotester. Let's hope it goes through this time. |
hah! dlang bot edited its comment when you reopened, very slick. |
Removed the auto merge label since the auto tester is going to merge it. |
Great. Thanks! |
https://issues.dlang.org/show_bug.cgi?id=17340