-
Notifications
You must be signed in to change notification settings - Fork 13
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 docs #65
Refactor docs #65
Conversation
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.
Looks good to me - I'm happy to have structure to the docs! I just wasn't sure about one issue with the routes section.
distance | ||
distances | ||
heighttoroot | ||
heightstoroot |
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.
These are definitely queries (above), but are they routes?
distance | |
distances | |
heighttoroot | |
heightstoroot |
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.
I guess not really, maybe we should move them - I have them here because they are defined in routes.jl
though.
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.
Yes, I suggest in #63 that you move them from routes.jl
to metrics.jl
now that metrics.jl
exists, so I think removing them from routes.md
makes sense too.
eebede1
to
7190c6e
Compare
I've kept all of your text except the bit where you say that the only thing this package really cares about is Diversity.jl |
Again, the errors seem to be from the grandparent, so apart from the |
df065e4
to
aeff34d
Compare
7190c6e
to
826e449
Compare
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
aeff34d
to
ded100d
Compare
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
Co-authored-by: Richard Reeve <git@richardreeve.net>
826e449
to
bcc0c98
Compare
@mkborregaard So, do you want to just delete those routes-that-aren't routes and merge this to reduce the complexity of the PR chain? |
Yeah I've actually done everything you asked but something seems to have gone wrong during the rebases. However the final version works, so let me just merge. |
Pull Request Test Coverage Report for Build 1341950236
💛 - Coveralls |
Richard, could you take a look at whether you are OK with me taking the docs this way? Maybe we could just merge all these tomorrow, then you can judge by looking at the docs -they this is of course in principle WIP.
I'd like to move forward with this in preparation of the course.