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

fix(affected): include not committed changes in --affected #9133

Merged
merged 8 commits into from
Sep 16, 2024

Conversation

NicholasLYang
Copy link
Contributor

Description

We were including new files that were not checked into git, but not changes to files already in git but not committed.

Testing Instructions

Added more testing to affected.t for both new files and existing files with new changes

Copy link

vercel bot commented Sep 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 16, 2024 6:28pm
8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Sep 16, 2024 6:28pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Sep 16, 2024 6:28pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Sep 16, 2024 6:28pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Sep 16, 2024 6:28pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Sep 16, 2024 6:28pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Sep 16, 2024 6:28pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Sep 16, 2024 6:28pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Sep 16, 2024 6:28pm

@NicholasLYang NicholasLYang enabled auto-merge (squash) September 10, 2024 19:41
Copy link
Member

@chris-olszewski chris-olszewski left a comment

Choose a reason for hiding this comment

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

I'm confused why --affected and --filter=[main] go down two different codepaths. Are we defaulting the SCM head for --affected too early?

You're more experienced in this area of the codebase so stamping to not delay the fix.

let output = self
.execute_git_command(&["ls-files", "--others", "--exclude-standard"], pathspec)?;
self.add_files_from_stdout(&mut files, turbo_root, output);

// Then add files that are in git, but have been modified (but not committed)
let output = self.execute_git_command(&["diff", "--name-only", "HEAD"], pathspec)?;
Copy link
Member

Choose a reason for hiding this comment

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

What if TURBO_SCM_HEAD is specified and isn't HEAD? Does this change behavior for SCM filters that specify a full git range like --filter=[commit-1...commit-2]?

I'm a little confused why we're duplicating the logic from the case where to_commit isn't specified? It seems like to_commit = None and include_uncommitted = true indicate the same thing?

@NicholasLYang NicholasLYang merged commit 9b9ca56 into main Sep 16, 2024
40 checks passed
@NicholasLYang NicholasLYang deleted the fix/affected-unstaged-changes branch September 16, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants