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

feat(docuploader): add destination-prefix option #26

Merged
merged 5 commits into from
Jul 13, 2020
Merged

feat(docuploader): add destination-prefix option #26

merged 5 commits into from
Jul 13, 2020

Conversation

tbpg
Copy link
Collaborator

@tbpg tbpg commented Jul 13, 2020

The prefix option will be used when uploading DocFX YAML. We need to
distinguish HTML docs vs DocFX YAML. Then, another job will convert the
YAML to HTML. This way, all DocFX tarballs can start with docfx- and we can
strip that prefix when uploading the resulting HTML.

The format option will be used when uploading DocFX YAML. We need to
distinguish HTML docs vs DocFX YAML. Then, another job will convert the
YAML to HTML.

In a future PR, I'll add the ability to edit the metadata to change the
format to HTML. That way, when we re-upload the result, it will have the
"correct" metadata format.
@tbpg tbpg requested review from tmatsuo and busunkim96 July 13, 2020 18:35
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 13, 2020
@tmatsuo
Copy link

tmatsuo commented Jul 13, 2020

I'm not sure if it's a good idea to have this option.

Instead can we always upload html files?

@JustinBeckwith
Copy link
Contributor

I love this idea. One of the original goals was to try getting to an intermediary format that could be re-generated into HTML as the template on cloudsite changes. By storing this in YAML instead of HTML, we can re-generate any time we want with different visuals.

@tmatsuo
Copy link

tmatsuo commented Jul 13, 2020

I'm not opposed to have a capability of only re-generating the HTML from yaml files.

I don't feel quite right to have this option and metadata.

How about just to have --destination-prefix option?

Copy link

@tmatsuo tmatsuo left a comment

Choose a reason for hiding this comment

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

I prefer --destination-prefix option

@tmatsuo
Copy link

tmatsuo commented Jul 13, 2020

I may miss the big picture, so I could be convinced if there's a reason to add format metadata.

@tbpg tbpg changed the title feat(docuploader): add format option feat(docuploader): add destination-prefix option Jul 13, 2020
@tbpg
Copy link
Collaborator Author

tbpg commented Jul 13, 2020

I switched it to --destination-prefix.

The risk of this is that each client library will be responsible for setting the right docfx prefix. Typos will not be caught when calling the tool -- only when docs don't show up. We also need to be consistent in how the prefix is applied, so we can remove it in the HTML generation job.

@tbpg tbpg requested a review from tmatsuo July 13, 2020 21:56
@tbpg tbpg merged commit 68176ed into master Jul 13, 2020
@tbpg tbpg deleted the format branch July 13, 2020 22:57
@tbpg tbpg mentioned this pull request Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants