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

infra(netlify): diff to next #2755

Merged
merged 3 commits into from
Mar 18, 2024
Merged

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Mar 15, 2024

Currently, if you merge next into a PR, a build preview is created, even if the PR itself does not change anything docs related.

The new check uses next as a comparison base and uses a common ancestor comparison.
E.g. any changes on next in the meantime are ignored.

git diff next...some-branch vs git diff next some-branch

Unfortunately, this will refresh the preview for all commits in a PR that changes docs, not just those that affect the docs.

So more build-minutes spend on PRs that would change the docs. And no more build-minutes spend on PRs that don't change the docs.

@ST-DDT ST-DDT added p: 1-normal Nothing urgent c: infra Changes to our infrastructure or project setup labels Mar 15, 2024
@ST-DDT ST-DDT added this to the vAnytime milestone Mar 15, 2024
@ST-DDT ST-DDT requested review from a team March 15, 2024 17:37
@ST-DDT ST-DDT self-assigned this Mar 15, 2024
Copy link

netlify bot commented Mar 15, 2024

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit 5f96368
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/65f855d81a74960009532248
😎 Deploy Preview https://deploy-preview-2755.fakerjs.dev
📱 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 configuration.

Copy link

codecov bot commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.93%. Comparing base (6dee178) to head (5f96368).

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2755      +/-   ##
==========================================
- Coverage   99.93%   99.93%   -0.01%     
==========================================
  Files        2958     2958              
  Lines      213715   213715              
  Branches      950      948       -2     
==========================================
- Hits       213583   213568      -15     
- Misses        132      143      +11     
- Partials        0        4       +4     

see 6 files with indirect coverage changes

@ST-DDT
Copy link
Member Author

ST-DDT commented Mar 16, 2024

Alternatively, we could check for a /^docs/ (+feat+refactor?) commit name.

Or maybe even a specific (empty) enable netlify commit.

I would have to check that though if it does strange things with update to next though.

@ST-DDT ST-DDT requested a review from a team March 16, 2024 08:33
@xDivisionByZerox xDivisionByZerox enabled auto-merge (squash) March 18, 2024 14:55
@xDivisionByZerox
Copy link
Member

Alternatively, we could check for a /^docs/ (+feat+refactor?) commit name.

Or maybe even a specific (empty) enable netlify commit.

I would have to check that though if it does strange things with update to next though.

KISS. If we need to change the process again, we can have a look if it's required.

@xDivisionByZerox xDivisionByZerox merged commit a27aafe into next Mar 18, 2024
20 checks passed
@ST-DDT ST-DDT deleted the infra/netlify/diff-to-next branch April 17, 2024 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: infra Changes to our infrastructure or project setup p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants