GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-100072: only trigger netlify builds for doc changes by sobolevn · Pull Request #100074 · python/cpython

Conversation

@sobolevn

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

Uh oh!

There was an error while loading. Please reload this page.

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

  • Issue: Netlify builds happen for PRs that do not change docs #100072

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sobolevn
pythongh-100072: only trigger netlify builds for doc changes
4bc124c
@sobolevn
sobolevn requested a review from ezio-melotti as a code owner December 7, 2022 13:26
@bedevere-bot bedevere-bot mentioned this pull request Dec 7, 2022
Netlify builds happen for PRs that do not change docs #100072
Closed
@netlify

netlify Bot commented Dec 7, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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...

QR Code

Use your smartphone camera to open QR code link.

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot added the awaiting review label Dec 7, 2022
@sobolevn sobolevn added the skip news label Dec 7, 2022
@hugovk

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:

  • DO NOT MERGE: non docs change hugovk/cpython#34
  • DO NOT MERGE: docs change hugovk/cpython#35

But Netlify built both.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sobolevn added 2 commits December 7, 2022 17:46
@sobolevn
Update netlify.toml
5e3d116
@sobolevn
Update doc.yml
3b5a8df
@sobolevn

sobolevn commented Dec 7, 2022

Copy link
Copy Markdown
Member Author

Hm, maybe like this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hugovk

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sobolevn

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sobolevn
Update netlify.toml
03c1dd1
@hugovk

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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk
hugovk reviewed Dec 7, 2022
View reviewed changes
Comment thread netlify.toml Resolved

Uh oh!

There was an error while loading. Please reload this page.

@sobolevn @hugovk
Update netlify.toml
35c4b75
Co-authored-by: Hugo van Kemenade <[email protected]>
@sobolevn

sobolevn commented Dec 7, 2022

Copy link
Copy Markdown
Member Author

Thanks, base is something I have completely missed!

hugovk reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk
hugovk approved these changes Dec 7, 2022
View reviewed changes

@hugovk hugovk left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thank you, let's give this a shot!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Dec 7, 2022
@sobolevn

sobolevn commented Dec 7, 2022

Copy link
Copy Markdown
Member Author

Thank you for testing and finding a solution ;)

hugovk reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hugovk
hugovk merged commit d92407e into python:main Dec 7, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Dec 7, 2022
@hugovk hugovk mentioned this pull request Dec 18, 2022
Docs: Don't upload CI artifacts #100330
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@sobolevn @hugovk @bedevere-bot
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.