The Wayback Machine - https://web.archive.org/web/20221219192922/https://github.com/python/cpython/pull/100330
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

Jump to bottom

Docs: Don't upload CI artifacts #100330

Merged
merged 1 commit into from Dec 19, 2022
Merged

Docs: Don't upload CI artifacts #100330

merged 1 commit into from Dec 19, 2022
+0 −10

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Dec 18, 2022

On the CI, we no longer need to upload the built docs as an artifact, as we now have previews on Netlify (added in #92852). For example:

This will save about 49s on the CI and 59.1 MB in storage.

We can also skip uploading the ~1 MB NEWS file because it's visible in Netlify previews, for example at:

@hugovk hugovk requested a review from CAM-Gerlach Dec 18, 2022
@hugovk hugovk requested a review from ezio-melotti as a code owner Dec 18, 2022
@hugovk
Copy link
Member Author

hugovk commented Dec 18, 2022

One difference:

The CI builds/uploads docs if there are changes in Doc/, Misc/ (home of the news fragments) or its own config (doc.yml):

cpython/.github/workflows/doc.yml

Lines 23 to 26 in 1cf3d78

paths:
- 'Doc/**'
- 'Misc/**'
- '.github/workflows/doc.yml'

Netlify only builds previews if there are changes in Doc/ or its own config (netlify.toml):

cpython/netlify.toml

Lines 2 to 7 in 1cf3d78

base = "Doc/"
command = "make html"
publish = "build/html"
# Do not trigger netlify builds if docs were not changed.
# Changed files should be in sync with `.github/workflows/doc.yml`
ignore = "git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF . ../netlify.toml"

So this PR didn't get a preview (that's fine), but nor would changes to Misc/.

Do we want to add Misc/ to Netlify builds?

(This was an open question in @sobolevn's #100074 that went unanswered.)

sobolevn
sobolevn approved these changes Dec 18, 2022
View changes
Copy link
Member

@sobolevn sobolevn left a comment •
edited

I agree, I don't think we need it anymore!

Do we want to add Misc/ to Netlify builds?

I, personally, don't see a use-case for this. News are simple, markup there is also simple.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Dec 18, 2022 •
edited

Do we want to add Misc/ to Netlify builds?

IMO, no, since it means every PR would get a Netlify build, basically, as most PRs except for docs ones get a NEWS entry, and it seems unlikely to be practically useful/checked, especially since they are short.

@CAM-Gerlach CAM-Gerlach added the docs Documentation in the Doc dir label Dec 18, 2022
CAM-Gerlach
CAM-Gerlach approved these changes Dec 18, 2022
View changes
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

LGTM, thanks @hugovk !

@hugovk hugovk merged commit 702a5bc into python:main Dec 19, 2022
16 checks passed
@hugovk hugovk deleted the docs-rm-artifacts branch Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sobolevn sobolevn

@CAM-Gerlach CAM-Gerlach

@ezio-melotti ezio-melotti

Assignees
No one assigned
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@hugovk @CAM-Gerlach @sobolevn @ezio-melotti @bedevere-bot