This repository was archived by the owner on Oct 23, 2023. It is now read-only.
Using RST for documentation generation#169
Merged
Conversation
* Added handler for build finished event to fix doc links * Added contributing guide * Adding wait for the subprocess * Fixed the path issue with finding the script for fixing the links * Added darwin switch for sed -i flag * Fixed non darwin with -i flag for sed * Using find instead of xargs as it fails on readthedocs * Moving the substitution errors to /dev/null * Escaped the Link values Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
…oto files and fixed doc issues Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
kumare3
reviewed
May 23, 2021
Contributor
kumare3
left a comment
kumare3
left a comment
There was a problem hiding this comment.
I would like if the make generate just works without the additional steps
|
|
||
| .. prompt:: bash | ||
|
|
||
| git clone https://github.com/paweld2/protoc-gen-doc.git |
Contributor
There was a problem hiding this comment.
Why do we need to do this? We can just reference the gitsha in go get right? Other option is to fork the repo into flyteorg and use the fork?
Contributor
Author
There was a problem hiding this comment.
Done. Using boiler plate code now and updated the documentation.
Also currently the gihub workflows don't call make generate and hence don't find any issues.
We will have to fix the github workflows to use it so that the DELTA_CHECK catches any new files or changes
Contributor
Author
|
@kumare3 we need those additional steps for doc generation |
Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
kumare3
approved these changes
May 24, 2021
9 tasks
wild-endeavor
approved these changes
May 24, 2021
eapolinario
pushed a commit
that referenced
this pull request
Sep 8, 2023
* Fix the links in generated html files * Added handler for build finished event to fix doc links * Added contributing guide * Adding wait for the subprocess * Fixed the path issue with finding the script for fixing the links * Added darwin switch for sed -i flag * Fixed non darwin with -i flag for sed * Using find instead of xargs as it fails on readthedocs * Moving the substitution errors to /dev/null * Escaped the Link values Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com> * Using protoc-gen-doc plugin directly Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com> * Removed the example message Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com> * Fixed datacatalog file generation Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com> * Using template to generate docs and also removed unused imports in proto files and fixed doc issues Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com> * Minor doc fix Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com> * Added dependency to protoc-gen in biolerplate module Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Using protoc gen doc RST support to make it simpler to generate docs and correctly link them internally
This avoids the post build script to fix the html links which was proposed and implemented here
Fix the links in generated html files #168
Fixed the documentation warning that arose of the usage of RST which required fixing the comments in the proto files
As part of this did clean up the proto files to remove the commented out validation code which was using
protoc-gen-validate. Once we officially support then the code can revived from older version instead of keeping commented out code in the repoAlso removed unused imports from the proto files which would generate warnings during the doc generation process
Fixed the referenced links to google.protobuf.[timestamp|Duration|struct] by generating there documentation as part of the core.rst . This doc is generated at the tail end of the core documentation and helps with not having dead links to these where they are referenced in our documentation
Using a template for all RST file generation except for core. This was done to exclude the Scalar value doc to be generated in each module of the documentation
https://github.com/pseudomuto/protoc-gen-doc/blob/ae63e7aba9ef91669b2bc76634b33feead0e4f1a/resources/markdown.tmpl#L98
The new template removes the Scalar value and is only generated during core module generation and hence other modules can refer to the scalar values from the core.
https://docs.flyte.org/projects/flyteidl/en/docs-staging/developing.html#docs-generation
Rest of the docs can be seen here
https://docs.flyte.org/projects/flyteidl/en/docs-staging/protos/index.html
Type
Are all requirements met?
Complete description
How did you fix the bug, make the feature etc. Link to any design docs etc
Tracking Issue
https://github.com/lyft/flyte/issues/
Follow-up issue
NA
OR
https://github.com/lyft/flyte/issues/