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

Add back in heading for P4Smith README file #4756

Conversation

jafingerhut
Copy link
Contributor

This makes the auto-generated doxygen index easier to distinguish this files from others, instead of merely being called "README" there.

@fruffy
Copy link
Collaborator

fruffy commented Jun 24, 2024

@fruffy
Copy link
Collaborator

fruffy commented Jun 24, 2024

Looks like the formatting is similarly broken in other places: https://github.com/jafingerhut/p4c/tree/add-readme-heading-for-better-doxygen-index/backends/p4tools/modules/testgen

@jafingerhut
Copy link
Contributor Author

jafingerhut commented Jun 24, 2024

I am open to anything that you find acceptable for the page itself, but it seems very poor to have the Doxygen index of articles include "README" for this file.

@AdarshRawat1 Any Doxygen configuration tricks you know of that might help here?

@AdarshRawat1
Copy link
Member

AdarshRawat1 commented Jun 24, 2024

I am open to anything that you find acceptable for the page itself, but it seems very poor to have the Doxygen index of articles include "README" for this file.

@AdarshRawat1 Any Doxygen configuration tricks you know of that might help here?

Issue here

The page name and title come from the file name or level one heading, and Standard Markdown does not support header labeling.

If we can remove the image, then this should sort the issue of header name.

@jafingerhut
Copy link
Contributor Author

@fruffy What do you think of Adarsh's idea of removing the P4 logo image, and the second heading, but keeping the new first one? The test-tools pass/fail badge would stay.

@fruffy
Copy link
Collaborator

fruffy commented Jun 24, 2024

@fruffy What do you think of Adarsh's idea of removing the P4 logo image, and the second heading, but keeping the new first one? The test-tools pass/fail badge would stay.

Not happy since the logo adds some flavor but it can't be helped. The clean documentation takes priority.

@fruffy fruffy added the documentation Topics related to compiler documentation. label Jun 24, 2024
@jafingerhut
Copy link
Contributor Author

Ugh. Forgot to add DCO to these commits. Will try to add them retroactively.

This makes the auto-generated doxygen index easier to distinguish this
files from others, instead of merely being called "README" there.

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Maybe not as nice of a visual look as what was there before, but
seems worth looking at to see if it is desirable.

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
since previous attempt to keep it messed up the CI badge when viewing
Github flavored Markdown on github.com

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
@jafingerhut jafingerhut force-pushed the add-readme-heading-for-better-doxygen-index branch from c1b24d9 to 8d37a3f Compare June 24, 2024 20:44
@fruffy
Copy link
Collaborator

fruffy commented Jun 24, 2024

DCO is still optional :) I agree it is quite annoying.

@jafingerhut
Copy link
Contributor Author

The instructions for adding DCO retroactively works, even if you forgot them for multiple commits. Good!

@fruffy
Copy link
Collaborator

fruffy commented Jun 25, 2024

Could also do this for P4Testgen?

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
@jafingerhut
Copy link
Contributor Author

Could also do this for P4Testgen?

Done

@AdarshRawat1
Copy link
Member

Not happy since the logo adds some flavor but it can't be helped. The clean documentation takes priority.

How about having P4 image on the right-hand side of the page like this ??
image

@fruffy
Copy link
Collaborator

fruffy commented Jun 25, 2024

Not happy since the logo adds some flavor but it can't be helped. The clean documentation takes priority.

How about having P4 image on the right-hand side of the page like this ?? image

The thing was that the P4 logo + Testgen creates P4Testgen, which is now lost.

@jafingerhut jafingerhut added this pull request to the merge queue Jun 25, 2024
Merged via the queue into p4lang:main with commit d95d1c4 Jun 25, 2024
17 checks passed
@jafingerhut jafingerhut deleted the add-readme-heading-for-better-doxygen-index branch June 25, 2024 13:43
AdarshRawat1 pushed a commit to AdarshRawat1/p4c that referenced this pull request Jun 25, 2024
* Add back in heading for P4Smith README file
This makes the auto-generated doxygen index easier to distinguish this
files from others, instead of merely being called "README" there.

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

* Trying out a thing where the P4 logo is put before the CI badge
Maybe not as nice of a visual look as what was there before, but
seems worth looking at to see if it is desirable.

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

* Removing logo
since previous attempt to keep it messed up the CI badge when viewing
Github flavored Markdown on github.com

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

* Also remove extra heading for P4Testgen

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

---------

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Adarsh <Adarshbunny293@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Topics related to compiler documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants