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: Links in Generated Documentation #4793

Merged
merged 8 commits into from
Jul 21, 2024

Conversation

AdarshRawat1
Copy link
Member

@AdarshRawat1 AdarshRawat1 commented Jul 10, 2024

For better Presentation of content in generated documentation.

Changes

  • Enabled GitHub Markdown Style TOC (i.e. TOC given in README was no functional in generated output.)
  • Generated Relative link fixes to non README files and PPTs/PDF.
  • Docs : Added P4 Coding Standard Philosophy to Docs
  • Fix : Sequence of IR introduction in docs
  • Fix: relative linking to subsections in Doxygen output

Links fixed

on Main Page [P4 Compiler Documentation]

  • Card links on "p4 Compiler Onboarding" and "Getting Started"

on Repository Page

on P4C Page

@AdarshRawat1 AdarshRawat1 marked this pull request as draft July 10, 2024 07:23
@fruffy fruffy added the documentation Topics related to compiler documentation. label Jul 10, 2024
@fruffy
Copy link
Collaborator

fruffy commented Jul 12, 2024

Is this ready for review?

@AdarshRawat1
Copy link
Member Author

Is this ready for review?

Filtering the repo to find more, to avoid extra PRs.

@jafingerhut
Copy link
Contributor

So the primary way that I know to review fixes for broken links is to wait until a PR like this is merged in, wait a little while until the doxygen task runs to generate new output from the changes, and then look at those.

I do not mind learning how to take a PR and locally, on my own development system, run doxygen on the changes and look at them on my local system. Are there step-by-step instructions for doing that somewhere?

@jafingerhut
Copy link
Contributor

This newly created issue summarizes my confusion with how to use doxygen on p4c: #4805

@AdarshRawat1 AdarshRawat1 changed the title Docs: Minor Link Fixes Fix: Links in Generated Documentation Jul 19, 2024
@AdarshRawat1
Copy link
Member Author

So the primary way that I know to review fixes for broken links is to wait until a PR like this is merged in, wait a little while until the doxygen task runs to generate new output from the changes, and then look at those.

I do not mind learning how to take a PR and locally, on my own development system, run doxygen on the changes and look at them on my local system. Are there step-by-step instructions for doing that somewhere?

Yes, you can test the PR locally by checking out this PR.

  • Fetch the PR by using git fetch upstream pull/PR_ID/head
    Example for this PR-
git fetch upstream pull/4793/head   
  • Check out
git checkout FETCH_HEAD

@AdarshRawat1 AdarshRawat1 marked this pull request as ready for review July 19, 2024 17:53
Copy link
Contributor

@jafingerhut jafingerhut left a comment

Choose a reason for hiding this comment

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

LGTM

@jafingerhut
Copy link
Contributor

Given that these changes are fairly small and focused, and that it is easier to see what is fixed in the docs once the changes are merged in (yes, I know I can try out the changes locally on my system from the PR, without it being merged in), I would like to see this merged in, so we can look for issues in the generated docs after these changes are merged.

If we end up with one or two extra PRs, I do not think that is a big issue, relative to knowing what we have generated from the latest main branch version of all files.

@jafingerhut jafingerhut added this pull request to the merge queue Jul 21, 2024
Merged via the queue into p4lang:main with commit 6d2b10a Jul 21, 2024
17 checks passed
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