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

#415: API-alphabetical from md to rst #417

Merged

Conversation

antoinemeyer5
Copy link
Collaborator

@antoinemeyer5 antoinemeyer5 commented Jun 14, 2023

Notes

Screenshots

image
image
image
image
image
image
image

@antoinemeyer5 antoinemeyer5 requested review from cz4rs and fnrizzi June 14, 2023 08:26
@fnrizzi
Copy link
Collaborator

fnrizzi commented Jun 14, 2023

ine 12 and line 14: These are exactly the same lines. I have to remove the duplication, isn't it

yes

@fnrizzi
Copy link
Collaborator

fnrizzi commented Jun 14, 2023

please make sure ALL links are correct - I will make a pass too, but just want to make sure you do that too

@antoinemeyer5 antoinemeyer5 marked this pull request as ready for review June 14, 2023 09:12
Copy link
Member

@crtrott crtrott left a comment

Choose a reason for hiding this comment

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

I don't think we can do the Subview thing this way. Otherwise looks good.

docs/source/API/alphabetical.rst Outdated Show resolved Hide resolved
@antoinemeyer5 antoinemeyer5 requested a review from crtrott July 5, 2023 08:06
@fnrizzi
Copy link
Collaborator

fnrizzi commented Jul 5, 2023

@antoinemeyer5 can you please describe the solution so that it is clear what is going on

@antoinemeyer5
Copy link
Collaborator Author

antoinemeyer5 commented Jul 5, 2023

There are two ways to create internal links with rst:

(1) simple link: `Subview <core/view/Subview_type.html>`_

The first solution (1) is simple, but makes no difference between the link name and its display. Internally, the link is called Subview and the display clickable by the user is Subview. In our case, there were several links with the same internal name, so we had a duplication warning.

(2) more customizable link:

.. declaration
.. |SubviewType| replace:: Subview
.. _SubviewType: core/view/Subview_type.html

.. use
|SubviewType|_

This second solution (2) does the same thing and display, but differentiates between:

  • the internal name of the link (here SubviewType),
  • the display of the link to the user (here Subview) and
  • the destination of the link.

Thanks to this, you can converse with the desired display and avoid internal duplication. You must always be careful not to have another link in this file whose name is SubviewType, otherwise the duplication warning will return.

With this solution, it's much simpler to have internal link names with a certain shape and a different display. This separates rst's internal logic from the user's display.

@antoinemeyer5 antoinemeyer5 requested a review from fnrizzi July 5, 2023 08:34
@fnrizzi
Copy link
Collaborator

fnrizzi commented Jul 5, 2023

ok makes sense to me

@nmm0
Copy link
Contributor

nmm0 commented Jul 19, 2023

BTW - In the future, we could potentially rely on the index since it's auto-generated

@fnrizzi
Copy link
Collaborator

fnrizzi commented Jul 19, 2023

BTW - In the future, we could potentially rely on the index since it's auto-generated

sounds a good idea

@crtrott crtrott merged commit 69b1b16 into kokkos:main Jul 19, 2023
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.

5 participants