FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-100072: only trigger netlify builds for doc changes by sobolevn · Pull Request #100074 · python/cpython · GitHub

/ cpython Public

gh-100072: only trigger netlify builds for doc changes - #100074

Merged
hugovk merged 5 commits into
python:mainfrom
sobolevn:issue-100072
Dec 7, 2022
ConversationCommits (5)ChecksFiles changed
Merged

gh-100072: only trigger netlify builds for doc changes#100074
hugovk merged 5 commits into
python:mainfrom
sobolevn:issue-100072

Conversation

sobolevn commented Dec 7, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

This PR will trigger the netlify build because of netlify.toml change, but hopefully it won't trigger changes anymore for other builds without ./Doc or ./Misc changes.

Open question: do we need ./Misc here? All NEWS go there, many PRs have it. But, I don't think we are really interested in rendering news. Opinions?

CC @Mariatta @hugovk @epicfaace

bedevere-bot mentioned this pull request Dec 7, 2022
Closed

netlify Bot commented Dec 7, 2022 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for python-cpython-preview ready!

Name Link
🔨 Latest commit 35c4b75
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/6390daa639c5580009d1c81d
😎 Deploy Preview https://deploy-preview-100074--python-cpython-preview.netlify.app
📱 Preview on mobile Toggle QR Code...



Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

hugovk commented Dec 7, 2022

Copy link
Copy Markdown
Member

Thanks! Looks like it needs a bit of tuning.

I added this to my fork's main, which has Netlify enabled: hugovk@71faec4

Then created two PRs inside my fork, one with a Lib/ change, another with a Doc/ change:

But Netlify built both.

sobolevn commented Dec 7, 2022

Copy link
Copy Markdown
Member Author

Hm, maybe like this?

hugovk commented Dec 7, 2022

Copy link
Copy Markdown
Member

Added that to my fork's main: hugovk@24060c3

Rebased the two PRs but both had Netlify builds.

sobolevn commented Dec 7, 2022

Copy link
Copy Markdown
Member Author

This command should work:

git diff --name-only | grep -q '^Doc/' && exit 1 || exit 0

An exit code of 1 indicates the contents have changed and the build process continues as usual. An exit code of 0 indicates that there are no changes and the build should stop.

Prep:

echo 'a' >> Doc/glossary.rst

Local tests (I use echo instead of exit for demo):

» git diff --name-only | grep -q '^Lib/' && echo 1 || echo 0
0
» git diff --name-only | grep -q '^Doc/' && echo 1 || echo 0
1

Do you agree?

hugovk commented Dec 7, 2022

Copy link
Copy Markdown
Member

With local testing, yes.

With Netlify, no...

hugovk@abbc30d shows "Deploy Preview canceled." for both PRs...

hugovk#34 with no docs change:

7:35:00 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --name-only | grep -q '^Doc/' && exit 1 || exit 0'
7:35:01 PM: User-specified ignore command returned exit code 0. Returning early from build.
7:35:01 PM: Creating deploy upload records
7:35:01 PM: Failed during stage 'checking build content for changes': Canceled build due to no content change
7:35:01 PM: Finished processing build request in 13.44133397s

hugovk#35 with docs change:

7:35:15 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --name-only | grep -q '^Doc/' && exit 1 || exit 0'
7:35:16 PM: User-specified ignore command returned exit code 0. Returning early from build.
7:35:16 PM: Creating deploy upload records
7:35:17 PM: Failed during stage 'checking build content for changes': Canceled build due to no content change
7:35:17 PM: Finished processing build request in 12.800699818s

Rewinding and looking at the logs for #100074 (comment):

hugovk#34 without docs change:

4:57:39 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF Doc/ netlify.toml'
4:57:40 PM: fatal: ambiguous argument 'Doc/': unknown revision or path not in the working tree.
4:57:40 PM: Use '--' to separate paths from revisions, like this:
4:57:40 PM: 'git <command> [<revision>...] -- [<file>...]'

hugovk#35 with docs change:

4:56:02 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF Doc/ netlify.toml'
4:56:02 PM: fatal: ambiguous argument 'Doc/': unknown revision or path not in the working tree.
4:56:02 PM: Use '--' to separate paths from revisions, like this:
4:56:02 PM: 'git <command> [<revision>...] -- [<file>...]'

So we've already set base = "Doc/", where:

The base directory setting prompts our buildbots to change to the specified directory to detect dependencies and perform caching during the build process. It’s useful for building from a monorepo or subdirectory.

So let's try git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF . ../netlify.toml: hugovk@1810da0

I also cleared the deploy caches, in case the rebase/force pushes

hugovk#34 without docs change - no Netlify build! ✅:

8:13:58 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF . ../netlify.toml'
8:13:58 PM: User-specified ignore command returned exit code 0. Returning early from build.
8:13:58 PM: Creating deploy upload records
8:13:58 PM: Failed during stage 'checking build content for changes': Canceled build due to no content change
8:13:59 PM: Finished processing build request in 37.864234653s

hugovk#35 with docs change - has Netlify build! ✅:

8:11:51 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF . ../netlify.toml'
8:11:51 PM: Section completed: initializing
8:11:51 PM: Starting build script
...

So let's try this!

hugovk reviewed Dec 7, 2022
View reviewed changes
Comment thread netlify.toml Outdated
Update netlify.toml
35c4b75 
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>

sobolevn commented Dec 7, 2022

Copy link
Copy Markdown
Member Author

Thanks, base is something I have completely missed!

hugovk approved these changes Dec 7, 2022
View reviewed changes

hugovk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thank you, let's give this a shot!

sobolevn commented Dec 7, 2022

Copy link
Copy Markdown
Member Author

Thank you for testing and finding a solution ;)

hugovk merged commit d92407e into python:main Dec 7, 2022
hugovk mentioned this pull request Dec 18, 2022
Merged
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

hugovk hugovk approved these changes
ezio-melotti Awaiting requested review from ezio-melotti

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL