Include random.gd before coll.gi #2205
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This way, we can move the definition of GlobalMersenneTwister from coll.gi to random.gd, and will be able to use InstallMethodWithRandomSource inside of coll.gi
fingolfin
added
topic: library
release notes: not needed
ChrisJefferson
approved these changes
Feb 23, 2018
|
This seems like a reasonable change, but it desn't merge well with my PR, so we should merge this, then I throw my PR away and start again. |
|
If you are willing to do that: fine by me, thanks, and sorry for the extra work |
|
BTW, I don't think it is that incompatible: the first commit of your PR should not conflict at all, and the second just needs the "a (finite) collection" method installation moved back to I could also do that, and add your commits to this PR, then you have no further work. Just let me know what you prefer (i.e. either merge this PR then rebase yours, or tell me to merge your work into this PR). |
|
I'll merge this and fix, my PR has other problems (some infinite loop is happening) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This way, we can move the definition of GlobalMersenneTwister from
coll.gi to random.gd, and will be able to use InstallMethodWithRandomSource
inside of coll.gi
This implements a change I suggested on #2204 -- if @ChrisJefferson likes it, also feel free to cherry-pick this into your PR, and then we just test and merge your PR and we close this. Or we merge this first. Or we discard this PR if people hate it, and do something completely different ;-).