Skip to content
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

Fix/docs2 #2805

Merged
merged 15 commits into from Sep 27, 2022
Merged

Fix/docs2 #2805

merged 15 commits into from Sep 27, 2022

Conversation

SteveBronder
Copy link
Collaborator

@SteveBronder SteveBronder commented Aug 16, 2022

Summary

I'm breaking up #2687 into several smaller pieces as the other PR was error'ing out in some odd places and that large of a PR is hard to debug. I think I'm going to do it like

  • Fix up documentation website
  • Fix up docs for meta functions like value_type_t etc.
  • Several small Prs to remove errors from argument 'x' from the argument list of <FUNCTION> has multiple @param documentation sections

Tests

Only doc changes so no new tests

Side Effects

Nope

Release notes

Update math library documentation

Checklist

  • Math issue #(issue number)

  • Copyright holder: Steve Bronder

    The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
    - Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
    - Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

  • the basic tests are passing

    • unit tests pass (to run, use: ./runTests.py test/unit)
    • header checks pass, (make test-headers)
    • dependencies checks pass, (make test-math-dependencies)
    • docs build, (make doxygen)
    • code passes the built in C++ standards checks (make cpplint)
  • the code is written in idiomatic C++ and changes are documented in the doxygen

  • the new changes are tested

@bob-carpenter
Copy link
Contributor

Thanks so much for finishing the revised PR.

I'm not going to review, because I don't understand the new math lib's C++ well enough. Ideally, someone would have the time to follow along and see if the instructions work. Barring that, I'd suggest someone who understands the C++ already. Perhaps @rok-cesnovar or @wds15 or @andrjohns?

@SteveBronder
Copy link
Collaborator Author

That's reasonable. If any of the three of yinz also have contributions that you think would help out the docs absolutely feel free to add to the branch!

@rok-cesnovar
Copy link
Member

Will review it this weekend! Thanks Steve!

Copy link
Member

@rok-cesnovar rok-cesnovar left a comment

Choose a reason for hiding this comment

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

Looks great to me. Some grammar/typo comments, but all are rather small. No objections in terms of content/organization.

README.md Show resolved Hide resolved
doxygen/README.md Outdated Show resolved Hide resolved
doxygen/README.md Outdated Show resolved Hide resolved
doxygen/README.md Outdated Show resolved Hide resolved
doxygen/README.md Outdated Show resolved Hide resolved
doxygen/contributor_help_pages/getting_started.md Outdated Show resolved Hide resolved
doxygen/contributor_help_pages/getting_started.md Outdated Show resolved Hide resolved
doxygen/contributor_help_pages/getting_started.md Outdated Show resolved Hide resolved
doxygen/contributor_help_pages/getting_started.md Outdated Show resolved Hide resolved
doxygen/contributor_help_pages/getting_started.md Outdated Show resolved Hide resolved
@WardBrian
Copy link
Member

Are there any objections to committing @rok-cesnovar's grammar suggestions and then merging this? I've had a few people recently ask me about getting started docs here

@bob-carpenter
Copy link
Contributor

Are there any objections to committing @rok-cesnovar's grammar suggestions and then merging this?

Yes, I think it's better than what we have now and I don't think @SteveBronder has much time to revise.

I gave Steve a pile of feedback on the text---I just didn't want to be a final reviewer because I don't understand the C++ well enough and don't have time to verify it all.

@SteveBronder
Copy link
Collaborator Author

Hey srry! Been busy ill look at this tmrw and make the changes

@WardBrian
Copy link
Member

No worries Steve! I wasn't asking to try to rush you, but more ask if you wanted someone else (e.g. me) to go through Rok's suggestions and then merge this

syclik and others added 7 commits September 15, 2022 10:38
Co-authored-by: Rok Češnovar <rok.cesnovar@fri.uni-lj.si>
Co-authored-by: Rok Češnovar <rok.cesnovar@fri.uni-lj.si>
Co-authored-by: Rok Češnovar <rok.cesnovar@fri.uni-lj.si>
Co-authored-by: Rok Češnovar <rok.cesnovar@fri.uni-lj.si>
Co-authored-by: Rok Češnovar <rok.cesnovar@fri.uni-lj.si>
Co-authored-by: Rok Češnovar <rok.cesnovar@fri.uni-lj.si>
@WardBrian
Copy link
Member

I just went through all of @rok-cesnovar's grammar and spelling line edits, so assuming he is okay with the content of this I believe we should merge after CI

rok-cesnovar
rok-cesnovar previously approved these changes Sep 22, 2022
@rok-cesnovar rok-cesnovar merged commit 142b09c into develop Sep 27, 2022
@rok-cesnovar rok-cesnovar deleted the fix/docs2 branch September 27, 2022 17:35
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.

None yet

6 participants