Skip to content

Conversation

devmotion
Copy link
Member

Hi!

I finally found the time to separate the naming changes and comments, together with some new additions, from #12. Moreover, I reformatted most parts of the code to follow the Julia guidelines for code contributions, since the same conventions, like e.g. line breaks after 92 characters, should apply to docstrings anyway.

Beside that, the only change to the code is the replacement of type with struct - or should these changes be moved to another PR?

I guess, the most comments are added to integrator_interface.jl and solve.jl - hopefully they can help for understanding the implementation.

"""
savevalues!(integrator::DDEIntegrator, force_save=false)
Update solution of `integrator`, if necessary or forced by `force_save`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the Julia documentation guide lines, "In general, only the most generic method should be documented" - so maybe this (and other docstrings in integrator_interface.jl) should be moved to OrdinaryDiffEq.jl rather?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to have it both places. I'll accept it here.

@coveralls
Copy link

coveralls commented Jul 7, 2017

Coverage Status

Coverage decreased (-1.2%) to 78.049% when pulling 0dc0635 on devmotion:comments into 47342df on JuliaDiffEq:master.

@codecov-io
Copy link

codecov-io commented Jul 7, 2017

Codecov Report

Merging #13 into master will decrease coverage by 1.21%.
The diff coverage is 79.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #13      +/-   ##
==========================================
- Coverage   79.26%   78.04%   -1.22%     
==========================================
  Files           8        8              
  Lines         164      164              
==========================================
- Hits          130      128       -2     
- Misses         34       36       +2
Impacted Files Coverage Δ
src/algorithms.jl 100% <100%> (ø) ⬆️
src/alg_utils.jl 100% <100%> (ø) ⬆️
src/utils.jl 100% <100%> (ø) ⬆️
src/callbacks.jl 100% <100%> (ø) ⬆️
src/integrator_interface.jl 59.74% <63.01%> (-1.6%) ⬇️
src/integrator_type.jl 66.66% <66.66%> (-8.34%) ⬇️
src/history_function.jl 93.33% <93.33%> (ø) ⬆️
src/solve.jl 95.16% <95.16%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47342df...0dc0635. Read the comment docs.

@ChrisRackauckas ChrisRackauckas merged commit 2f3ffbb into SciML:master Jul 7, 2017
@devmotion devmotion deleted the comments branch July 8, 2017 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants