-
-
Notifications
You must be signed in to change notification settings - Fork 700
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 16291 -- phobosinit fails to register encodings on individual tests #4743
Conversation
|
@schveiguy, thanks for your PR! By analyzing the annotation information on this pull request, we identified @tchaloupka, @andralex and @lionello to be potential reviewers. @tchaloupka: The PR was automatically assigned to you, please reassign it if you were identified mistakenly. |
ctors back in std.encoding.
@schveiguy, thanks for your PR! By analyzing the annotation information on this pull request, we identified @JackStouffer, @kyllingstad and @andralex to be potential reviewers. @JackStouffer: The PR was automatically assigned to you, please reassign it if you were identified mistakenly. |
Current coverage is 88.78% (diff: 100%)@@ master #4743 diff @@
==========================================
Files 121 120 -1
Lines 74159 74151 -8
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 65845 65837 -8
Misses 8314 8314
Partials 0 0
|
lol apparently this was supposed to be assigned to me, but since I'm not part of this repo it did nothing. |
@schveiguy Why not just use git to revert the commit that made this change? |
The bot has issues :) dlang-bots/mention-bot#7
The commit was more than just this, including renaming processinit to phobosinit (which I think is more appropriate if we have to add more of this shit). I don't want to revert all that. |
OK, the windows issue is the one that didn't fail on OSX. I need to figure out that bug (there is definitely a cycle, as I recall, but it's just not detected on some OSes), and also rearrange how this is done so the phobosinit gets pulled in. Stay tuned. |
I'm going to attempt moving these ctors back, and make sure the dependency graph is properly set up. Testing locally, I no longer have any cycles. The cycle detection code is now fixed, so if cycles return, I'll have to figure out a different way to solve this.
Please do NOT pull until the auto-tester has checked for cycles.
See related PRs for history: dlang/druntime#1602 and #4493