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

[API Reference docs] Remove git references from GitHub Action templates #813

Merged
merged 3 commits into from
Mar 31, 2022

Conversation

LysandreJik
Copy link
Member

@LysandreJik LysandreJik commented Mar 31, 2022

This PR fixes the build_documentation as well as other doc building actions by removing the git references used previously. These were references to branches that have since been merged in the default branch.

Closes #811

@LysandreJik LysandreJik requested a review from adrinjalali March 31, 2022 10:11
@LysandreJik LysandreJik changed the title [API Reference docs] Fix build_documentation Action [API Reference docs] Remove git references from GitHub Action templates Mar 31, 2022
@LysandreJik
Copy link
Member Author

The reference has been set to main as the templates need a specific revision set in order to work (see error otherwise).

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Mar 31, 2022

The documentation is not available anymore as the PR was closed or merged.

@adrinjalali
Copy link
Contributor

@LysandreJik can we replace the comment left by @HuggingFaceDocBuilderDev by a github action whose Details would link to the artifacts? It's adding too much noise to the PRs IMO.

@adrinjalali
Copy link
Contributor

Should the reference be main or the PR? Ideally we'd like the CI on main to build and publish the docs, and the PRs to build the docs for the PR (ideally only the changed files)

@LysandreJik
Copy link
Member Author

@LysandreJik can we replace the comment left by @HuggingFaceDocBuilderDev by a github action whose Details would link to the artifacts? It's adding too much noise to the PRs IMO.

I'd rather we stay coherent with the rest of the HF libraries which all provide the docs in a very accessible manner. Could you show me what you have in mind with a GitHub Action whose details would link to the artifacts? I believe we had something similar with CircleCI in the past, but none of the users were aware of it even if mentioned in the contributing guide.

Should the reference be main or the PR? Ideally we'd like the CI on main to build and publish the docs, and the PRs to build the docs for the PR (ideally only the changed files)

The reference specified here is the reference to doc-builder. Here it will use the workflows defined on the main branch of doc-builder.

The CI currently does the following:

@adrinjalali
Copy link
Contributor

This is how it could look like, which you can see in this PR (scikit-learn/scikit-learn#23002), and it only compiles the changed documents, with links to the PR version vs the main (dev) version vs the latest stable release (we really don't have to implement all of that here). The Details on the ci/circleci: doc artifact action directly points to where the built documentation sits:

Screenshot from 2022-03-31 14-10-44

The solution right now adds a comment (at least one I guess) to every PR, that's a ton of noise. HuggingFace is not a single community, and I think the community of each project should decide how to configure the tools they use rather than it being one solution for all.

@LysandreJik
Copy link
Member Author

I understand. Would you be open to discussing this in a separate issue, as it's not linked to this PR; and to merge this PR so that we may have the docs deployed?

@LysandreJik LysandreJik merged commit 8a43817 into huggingface:main Mar 31, 2022
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.

[API Reference docs] The build_documentation Action does not run successfully
3 participants