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

feat: git change detection works on any commit #1235

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

snakster
Copy link
Contributor

@snakster snakster commented Nov 15, 2023

Reason for This Change

Currently, the Git revision that's used for change comparison with terramate (run | list) --changed is selected based on these rules (simplified):

  • If HEAD commit == origin/main, select previous commit
  • Otherwise, select origin/main

This means for the two cases of either getting a preview of a deployment, or executing the deployment, it will select the changed stacks relative to the previous deployment (w.r.t. to the commit graph).

For anything before origin/main, it will not select the changed stacks relative to the previous deployment, but the changed stacks compared to the latest deployment.

This is inconsistent, and more importantly, can lead to unintended behaviour in case of queued CI jobs that are behind origin/main when multiple PRs are merged in parallel.

Description of Changes

The selection logic for the compared base revision is changed so that "select the changed stacks relative to the previous deployment"-behaviour works on any commit.

Simplified, this means any commit is now compared its fork point from origin/main.

Copy link

netlify bot commented Nov 15, 2023

Deploy Preview for terramate-io-docs ready!

Name Link
🔨 Latest commit 14158a3
🔍 Latest deploy log https://app.netlify.com/sites/terramate-io-docs/deploys/655c8382720fac0008936555
😎 Deploy Preview https://deploy-preview-1235--terramate-io-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 91 (🟢 up 1 from production)
Accessibility: 97 (🔴 down 3 from production)
Best Practices: 92 (🟢 up 9 from production)
SEO: 100 (🟢 up 5 from production)
PWA: 60 (🟢 up 20 from production)
View the detailed breakdown and full score reports

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

@snakster snakster force-pushed the snk-git-change-detection branch 2 times, most recently from ab25e41 to 318124b Compare November 15, 2023 01:30
Copy link

github-actions bot commented Nov 15, 2023

metric: time/op
CloudReadLines-4: old 1.04ms ± 5%: new 1.02ms ± 4%: delta: -2.54%
CloudReadLine-4: old 7.34ms ± 2%: new 7.25ms ± 1%: delta: -1.26%
ListFiles-4: old 53.2µs ± 2%: new 53.1µs ± 1%: delta: 0.00%
Generate-4: old 2.65s ± 3%: new 2.65s ± 2%: delta: 0.00%
GenerateRegex-4: old 1.85s ± 3%: new 1.89s ± 2%: delta: 2.14%
TokensForExpressionComplex-4: old 1.29ms ± 1%: new 1.29ms ± 1%: delta: 0.00%
TokensForExpressionPlainStringNoNewline-4: old 901ns ± 2%: new 909ns ± 1%: delta: 1.00%
TokensForExpressionStringWith100Newlines-4: old 22.1µs ± 2%: new 22.6µs ± 1%: delta: 1.91%
TokensForExpressionObjectWith100KeysWithNumberValues-4: old 1.48ms ± 1%: new 1.48ms ± 1%: delta: 0.29%
TokensForExpression-4: old 1.29ms ± 1%: new 1.28ms ± 1%: delta: 0.00%
PartialEvalComplex-4: old 443µs ± 1%: new 445µs ± 2%: delta: 0.00%
PartialEvalSmallString-4: old 3.68µs ± 1%: new 3.70µs ± 1%: delta: 0.57%
PartialEvalHugeString-4: old 1.89ms ± 0%: new 1.89ms ± 0%: delta: 0.00%
PartialEvalHugeInterpolatedString-4: old 5.02ms ± 1%: new 5.06ms ± 2%: delta: 0.78%
PartialEvalObject-4: old 22.0µs ± 2%: new 22.8µs ± 3%: delta: 3.98%
TmAllTrueLiteralList-4: old 6.34ms ± 1%: new 6.37ms ± 2%: delta: 0.00%
TmAllTrueFuncall-4: old 164µs ± 1%: new 163µs ± 2%: delta: -0.53%
TmAnyTrueLiteralList-4: old 149ms ± 1%: new 149ms ± 1%: delta: -0.40%
TmAnyTrueFuncall-4: old 164µs ± 1%: new 162µs ± 1%: delta: -0.83%
TmTernary-4: old 2.86µs ± 1%: new 2.84µs ± 1%: delta: -0.72%
TmTry-4: old 52.6µs ± 1%: new 52.5µs ± 1%: delta: -0.26%
metric: alloc/op
CloudReadLines-4: old 3.12MB ± 0%: new 3.12MB ± 0%: delta: 0.00%
CloudReadLine-4: old 3.37MB ± 0%: new 3.37MB ± 0%: delta: 0.00%
ListFiles-4: old 22.0kB ± 0%: new 22.0kB ± 0%: delta: 0.00%
Generate-4: old 2.32GB ± 0%: new 2.32GB ± 0%: delta: 0.00%
GenerateRegex-4: old 956MB ± 0%: new 956MB ± 0%: delta: 0.00%
TokensForExpressionComplex-4: old 412kB ± 0%: new 412kB ± 0%: delta: 0.00%
TokensForExpressionPlainStringNoNewline-4: old 592B ± 0%: new 592B ± 0%: delta: 0.00%
TokensForExpressionStringWith100Newlines-4: old 12.4kB ± 0%: new 12.4kB ± 0%: delta: 0.00%
TokensForExpressionObjectWith100KeysWithNumberValues-4: old 402kB ± 0%: new 402kB ± 0%: delta: 0.00%
TokensForExpression-4: old 412kB ± 0%: new 412kB ± 0%: delta: 0.00%
PartialEvalComplex-4: old 356kB ± 0%: new 356kB ± 0%: delta: 0.00%
PartialEvalSmallString-4: old 1.74kB ± 0%: new 1.74kB ± 0%: delta: 0.00%
PartialEvalHugeString-4: old 166kB ± 0%: new 166kB ± 0%: delta: 0.00%
PartialEvalHugeInterpolatedString-4: old 4.38MB ± 0%: new 4.38MB ± 0%: delta: 0.00%
PartialEvalObject-4: old 20.4kB ± 0%: new 20.4kB ± 0%: delta: 0.00%
TmAllTrueLiteralList-4: old 1.74MB ± 0%: new 1.74MB ± 0%: delta: 0.00%
TmAllTrueFuncall-4: old 45.5kB ± 0%: new 45.5kB ± 0%: delta: -0.00%
TmAnyTrueLiteralList-4: old 37.9MB ± 0%: new 37.9MB ± 0%: delta: -0.00%
TmAnyTrueFuncall-4: old 45.6kB ± 0%: new 45.6kB ± 0%: delta: -0.00%
TmTernary-4: old 1.20kB ± 0%: new 1.20kB ± 0%: delta: 0.00%
TmTry-4: old 11.2kB ± 0%: new 11.2kB ± 0%: delta: 0.00%
metric: allocs/op
CloudReadLines-4: old 5.54k ± 0%: new 5.54k ± 0%: delta: 0.00%
CloudReadLine-4: old 60.0k ± 0%: new 60.0k ± 0%: delta: 0.00%
ListFiles-4: old 321 ± 0%: new 321 ± 0%: delta: 0.00%
Generate-4: old 25.9M ± 0%: new 25.9M ± 0%: delta: 0.00%
GenerateRegex-4: old 18.6M ± 0%: new 18.6M ± 0%: delta: 0.00%
TokensForExpressionComplex-4: old 4.93k ± 0%: new 4.93k ± 0%: delta: 0.00%
TokensForExpressionPlainStringNoNewline-4: old 21.0 ± 0%: new 21.0 ± 0%: delta: 0.00%
TokensForExpressionStringWith100Newlines-4: old 228 ± 0%: new 228 ± 0%: delta: 0.00%
TokensForExpressionObjectWith100KeysWithNumberValues-4: old 3.29k ± 0%: new 3.29k ± 0%: delta: 0.00%
TokensForExpression-4: old 4.93k ± 0%: new 4.93k ± 0%: delta: 0.00%
PartialEvalComplex-4: old 2.86k ± 0%: new 2.86k ± 0%: delta: 0.00%
PartialEvalSmallString-4: old 23.0 ± 0%: new 23.0 ± 0%: delta: 0.00%
PartialEvalHugeString-4: old 35.0 ± 0%: new 35.0 ± 0%: delta: 0.00%
PartialEvalHugeInterpolatedString-4: old 23.1k ± 0%: new 23.1k ± 0%: delta: 0.00%
PartialEvalObject-4: old 125 ± 0%: new 125 ± 0%: delta: 0.00%
TmAllTrueLiteralList-4: old 13.6k ± 0%: new 13.6k ± 0%: delta: 0.00%
TmAllTrueFuncall-4: old 460 ± 0%: new 460 ± 0%: delta: 0.00%
TmAnyTrueLiteralList-4: old 252k ± 0%: new 252k ± 0%: delta: 0.00%
TmAnyTrueFuncall-4: old 462 ± 0%: new 462 ± 0%: delta: 0.00%
TmTernary-4: old 28.0 ± 0%: new 28.0 ± 0%: delta: 0.00%
TmTry-4: old 147 ± 0%: new 147 ± 0%: delta: 0.00%

@snakster snakster force-pushed the snk-git-change-detection branch 7 times, most recently from 123e97a to 767bcf8 Compare November 20, 2023 09:47
@snakster snakster changed the title fix: git change detection works on any commit feat: git change detection works on any commit Nov 20, 2023
cmd/terramate/cli/project.go Outdated Show resolved Hide resolved
@snakster snakster force-pushed the snk-git-change-detection branch from 767bcf8 to 14158a3 Compare November 21, 2023 10:16
Copy link
Contributor

@mariux mariux left a comment

Choose a reason for hiding this comment

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

finally tested the PR, please check the notion document for comments on a case this PR now creates wrong results when calculating changes.

@snakster snakster force-pushed the snk-git-change-detection branch from 14158a3 to d6d7d1d Compare December 28, 2023 23:57
Copy link

netlify bot commented Dec 28, 2023

Deploy Preview for docs-terramate-io canceled.

Name Link
🔨 Latest commit d3f17fc
🔍 Latest deploy log https://app.netlify.com/sites/docs-terramate-io/deploys/65fdc412cafda000088839be

@snakster snakster force-pushed the snk-git-change-detection branch 4 times, most recently from 9b7088b to c6895ee Compare January 2, 2024 11:29
@snakster snakster force-pushed the snk-git-change-detection branch 2 times, most recently from ddfe460 to 99a1ed3 Compare January 11, 2024 14:10
@soerenmartius
Copy link
Contributor

@mariux @snakster this needs to well documented also

@snakster snakster added this to the Release v0.5.0 milestone Jan 17, 2024
@snakster snakster added the enhancement New feature or request label Feb 8, 2024
@snakster snakster force-pushed the snk-git-change-detection branch from 99a1ed3 to 39f0dd0 Compare March 22, 2024 17:23
@snakster snakster removed the enhancement New feature or request label Mar 22, 2024
@snakster snakster force-pushed the snk-git-change-detection branch 2 times, most recently from f9f3a79 to 9815ff7 Compare March 22, 2024 17:37
@snakster snakster force-pushed the snk-git-change-detection branch from 9815ff7 to d3f17fc Compare March 22, 2024 17:46
@snakster snakster force-pushed the snk-git-change-detection branch from d3f17fc to 04e123d Compare May 10, 2024 18:39
@snakster snakster force-pushed the snk-git-change-detection branch from 04e123d to 3c0d658 Compare May 10, 2024 18:40
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.

4 participants