search

SciML / SBMLToolkit.jl

SBML differential equation and chemical reaction model (Gillespie simulations) for Julia's SciML ModelingToolkit
https://docs.sciml.ai/SBMLToolkit/stable/
MIT License
41 stars 10 forks source link

Pl refactor v0.1.13 #71

Closed paulflang closed 2 years ago

paulflang commented 2 years ago

This will need SBML#master, which is several commits ahead of v0.10.1

codecov[bot] commented 2 years ago

Codecov Report

Merging #71 (cb6ae1a) into main (08230bf) will increase coverage by 9.94%. The diff coverage is 98.67%.

@@            Coverage Diff             @@
##             main      #71      +/-   ##
==========================================
+ Coverage   88.67%   98.61%   +9.94%     
==========================================
  Files           1        1              
  Lines         256      217      -39     
==========================================
- Hits          227      214      -13     
+ Misses         29        3      -26
Impacted Files Coverage Δ
src/reactionsystem.jl 98.61% <98.67%> (+9.94%) :arrow_up:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

paulflang commented 2 years ago

src/reactionsystems.jl has become a rather big file. I am planning to split it up in multiple files in a future PR.

anandijain commented 2 years ago

otherwise LGTM but make sure to squash and merge

paulflang commented 2 years ago

Thanks! Squash and merge messes with history, though.

anandijain commented 2 years ago

there is only 109 commits total, this PR has 81. do you want this patch revision to account for more almost half of the entire repository? i dont

paulflang commented 2 years ago

@ChrisRackauckas what is the philosophy regarding clean vs accurate commit history (squash merge vs merge) in SciML? All I see in the ColPracs on "squash" is

You should not squash down commits while review is still on-going. Squashing commits prevents the reviewer being able to see what commits are added since the last review.

paulflang commented 2 years ago

Decided to merge without squashing, in case sth comes up that we need to revert a commit again, like 2 days ago.