-
-
Notifications
You must be signed in to change notification settings - Fork 934
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
refactor!: reorganize src folder #909
Conversation
Codecov Report
@@ Coverage Diff @@
## main #909 +/- ##
==========================================
- Coverage 99.13% 99.13% -0.01%
==========================================
Files 1958 1958
Lines 210788 210793 +5
Branches 907 906 -1
==========================================
- Hits 208971 208964 -7
- Misses 1741 1753 +12
Partials 76 76
|
While we at it: Should we provide a directory for each module? Like |
This could be a bit overkill for most modules for now, but should give more reliability in the future (perhaps, IDK TBH) since files can be added more easily. |
I don't think we should do it for all modules, but maybe for all that would benefit from it? |
The difference would be that we would not lose file history again if we need to reorganize since all files are already in their own module directory. |
To be fair, I had the same idea as @xDivisionByZerox 🤔 |
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.
Please rename src/modules/mersenne/mersenne.ts
to something like src/modules/mersenne/twister.ts
to reduce some confusion.
Sure! It should be done in a separate PR. I was just giving a rationale why its worth to have subfolders. |
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.
Do we need to change the test directory at all? Otherwise, the PR title could be confusing. So either revert the test changes from the current PR or change the title, please.
Yeah, moving the tests twice is kind of redundant. |
I extracted the test restructure into its own issue: #911 |
No description provided.