-
Notifications
You must be signed in to change notification settings - Fork 605
discard changes #7597
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
discard changes #7597
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@Byron is attempting to deploy a commit to the GitButler Team on Vercel. A member of the Team first needs to authorize it. |
Right, thanks for the reminder! I remember the dead-lock problem that taught us that lesson 😅. |
For now, no hunks or hunk-ranges.
That way the discard implementation can be triggered in the real world.
It seems to take quite some time on CI even though it's only for benches, which already have a feature toggle. Also remove unused dependencies everywhere.
…-space. We run out of disk-space so often now that the system is barely usable. To keep the change minimal, this is only applied to Rust checks.
It's an interesting side-effect that running in Docker seems to work better, maybe due to running as root? Try to emulate what seems to be happening on CI by changing the desired umask, which usually does the right thing when running locally.
Looking great, thanks Byron! |
) -> Result<Vec<but_workspace::discard::ui::DiscardSpec>, Error> { | ||
let project = projects.get(project_id)?; | ||
let repo = but_core::open_repo(&project.worktree_path())?; | ||
let _guard = project.exclusive_worktree_access(); |
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.
This implementation is made around "discard only uses Git and probably there aren't issues with parallelism", so it doesn't take the write-permission itself (yet). The CLI doesn't obtain it, for instance, for that reason.
Something I thought about is to create a second, slightly higher-level function of discard
that basically just takes a write-permission as argument, to nudge callers to use that instead. It felt, however, that it's enough to do that in tauri
for now.
I could totally understand that there should rather be the habit of passing write permissions, even though I usually avoid any extras on the function under test, so a separate one would be easier to handle.
Add V3 facilities for discarding changes in the worktree or index.
Tasks
.git/index
needs to represent the worktree) as neededgix
to point tomain
tauri
-integration to satisfy current UI capabilitiesrestore rust-test parallelism- no need, the cache works well and a full test-run is now below 5 minutesNext PR
Notes for the Reviewer
I flattened the Rust test setup, removing indirections and seemingly speeding it up, while reducing the chance that the runner runs out of disk space. For this PR, this now happened all the time, probably because running through docker consumes more disk space? Having a 'dumber' CI setup also seems like a win to me, despite less elegant handling of dependencies.
2419 minutes, and as less jobs are run more PRs can run in parallel.It turns out that the frequent CI failures were caused by disguised out-of-disk-space errors. They appear to be an LLVM-LD crash, but apparently that is just a C-Programs way of saying that the disk is full.
Shortcomings
Out of Scope
Research
unapply_lines
,unapply_ownership
andreset_files