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.

Sign up for GitHub

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

Jump to bottom

Fix docs build error and warnings #160

Merged
merged 4 commits into from
Dec 14, 2023
Merged

Fix docs build error and warnings #160

merged 4 commits into from
Dec 14, 2023

Conversation

neon60
Copy link
Contributor

@neon60 neon60 commented Nov 29, 2023 •
edited
Loading

@parbenc please continue, fix all warnings.

@neon60
Copy link
Contributor Author

neon60 commented Nov 30, 2023

@samjwu There are some overlap between your spelling branch and our branch. Can you check our changes and suggest if we should take over your commit(s)? We can also add the changes to your branch, if you want.

@samjwu
Copy link
Member

samjwu commented Nov 30, 2023

@samjwu There are some overlap between your spelling branch and our branch. Can you check our changes and suggest if we should take over your commit(s)? We can also add the changes to your branch, if you want.

I think it might be clearest to have both PRs to focus on separate concerns. It looks like this one fixes some things with doxygen while mine only does spelling.

@cgmillette cgmillette added documentation Improvements or additions to documentation ci:docs-only Skip most non-doc CI checks for this PR labels Dec 5, 2023
samjwu
samjwu previously approved these changes Dec 7, 2023
View reviewed changes
@neon60
Copy link
Contributor Author

neon60 commented Dec 11, 2023 •
edited
Loading

@samjwu There are some overlap between your spelling branch and our branch. Can you check our changes and suggest if we should take over your commit(s)? We can also add the changes to your branch, if you want.

I think it might be clearest to have both PRs to focus on separate concerns. It looks like this one fixes some things with doxygen while mine only does spelling.

You are right. We will have separate PRs and your PR already merged in.

@parbenc
fix build warnings
28fe756 
removed double std namespaces
fix underlines
@parbenc
Copy link
Contributor

parbenc commented Dec 12, 2023

Separated spellcheck stuff into another branch.

@neon60
Copy link
Contributor Author

neon60 commented Dec 13, 2023 •
edited
Loading

Separated spellcheck stuff into another branch.

@parbenc Can you open that PR?

@neon60
Copy link
Contributor Author

neon60 commented Dec 13, 2023 •
edited
Loading

@parbenc Can we remove the WIP from the title?

@parbenc
Copy link
Contributor

parbenc commented Dec 13, 2023

@parbenc Can we remove the WIP from the title?

Unfortunately i can't edit this PR.

@neon60 neon60 changed the title WIP - Fix Docs build error Fix Docs build error Dec 13, 2023
@neon60 neon60 changed the title Fix Docs build error Fix Docs build error and warnings Dec 13, 2023
@neon60 neon60 changed the title Fix Docs build error and warnings Fix docs build error and warnings Dec 13, 2023
@neon60
Copy link
Contributor Author

neon60 commented Dec 13, 2023

Looks good to me.

@parbenc
Copy link
Contributor

parbenc commented Dec 13, 2023

@cgmillette could you please look at this?

cgmillette
cgmillette reviewed Dec 13, 2023
View reviewed changes
Copy link
Collaborator

@cgmillette cgmillette left a comment

Choose a reason for hiding this comment

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

LGTM - just not sure about the dangling endcond

@cgmillette cgmillette self-requested a review December 14, 2023 16:24
@neon60 neon60 merged commit bcd81bd into ROCm:develop Dec 14, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@parbenc parbenc parbenc left review comments

@saadrahim saadrahim saadrahim approved these changes

@cgmillette cgmillette cgmillette approved these changes

@samjwu samjwu samjwu left review comments

@bragadeesh bragadeesh Awaiting requested review from bragadeesh bragadeesh is a code owner

@mkarunan mkarunan Awaiting requested review from mkarunan

@dlangbe dlangbe Awaiting requested review from dlangbe dlangbe is a code owner

@CongMa13 CongMa13 Awaiting requested review from CongMa13 CongMa13 is a code owner

@afanfa afanfa Awaiting requested review from afanfa afanfa is a code owner

@LisaDelaney LisaDelaney Awaiting requested review from LisaDelaney

Assignees
No one assigned
Labels
ci:docs-only Skip most non-doc CI checks for this PR documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@neon60 @samjwu @parbenc @saadrahim @cgmillette