-
Notifications
You must be signed in to change notification settings - Fork 11
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
split parameters.jl into smaller files #47
split parameters.jl into smaller files #47
Conversation
Codecov Report
@@ Coverage Diff @@
## master #47 +/- ##
==========================================
- Coverage 96.80% 96.55% -0.26%
==========================================
Files 4 7 +3
Lines 188 174 -14
==========================================
- Hits 182 168 -14
Misses 6 6
Continue to review full report at Codecov.
|
@willtebbutt would you be happy to accept this in principle? then I'll go through and sort out the merge conflict.... |
Sorry for the slow response @st-- . I like this PR, would be happy to merge it once merge conflicts have been resolved. |
@willtebbutt done, ready for review! |
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. Please could you bump the patch (I know this is meant to be a zero-modifcation PR, but just in case something has been missed), then I'll merge + tag.
I literally just moved code around - I even kept the order... Not really
something that needs to be pushed downstream right away, no? Can you not
just merge it as is, by the time I'm next on my laptop I'll have forgotten
and then before you know it there's a merge conflict with some new pr..
On Saturday, November 27, 2021, willtebbutt ***@***.***> wrote:
@willtebbutt approved this pull request.
LGTM. Please could you bump the patch (I know this is meant to be a
zero-modifcation PR, but just in case something has been missed), then I'll
merge + tag.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android. <
|
Resolves #44. Once we've agreed with the structure proposed in this PR, I'll also update the test/ structure to match.