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

[5/7] feat(submit:phabricator): do not abort entire process on failure #1202

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

arxanas
Copy link
Owner

@arxanas arxanas commented Jan 28, 2024

Stack:


feat(submit:phabricator): do not abort entire process on failure

Currently, git submit for Phabricator will abort the entire operation if any commit fails to be submitted. This means that if arc diff succeeds on one commit and then fails on its child, the entire operation is aborted. However, the first arc diff had side effects, so the user gets diffs uploaded to Phabricator that are not reflected locally. Instead, we should confirm any passing commits and abort after we get a failing commit. This commit updates the Phabricator forge to handle the error case better and not produce garbage commits on Phabricator.

@arxanas arxanas changed the title feat(submit:phabricator): do not abort entire process on failure [1/1] feat(submit:phabricator): do not abort entire process on failure Feb 27, 2024
@arxanas arxanas force-pushed the arxanas/phabricator branch from da5e8f5 to 70ae23b Compare February 27, 2024 22:39
@arxanas arxanas changed the title [1/1] feat(submit:phabricator): do not abort entire process on failure [5/7] feat(submit:phabricator): do not abort entire process on failure Jun 30, 2024
@arxanas arxanas changed the base branch from master to arxanas/feat-submit-phabricator-pass-allow-untracked-to-arc June 30, 2024 20:39
Base automatically changed from arxanas/feat-submit-phabricator-pass-allow-untracked-to-arc to master July 2, 2024 14:47
@arxanas arxanas force-pushed the arxanas/phabricator branch from 70ae23b to 8c36f0b Compare July 6, 2024 18:39
arxanas added 3 commits July 7, 2024 11:23
It seems easier for contributors to get started if the compiler version is already configured; plus, this will presumably make it so that I don't have to set the Rust version with `rustup override` whenever I create a new worktree.
This should have been done from the beginning, since we operate on these symbols in several places for formatting. This would also help a migration away from `cursive`, since now only two modules have direct dependencies on `cursive`/`cursive_core`:

- git-branchless-lib for general output formatting.
- git-branchless-undo for the interactive undo UI.
Currently, `git submit` for Phabricator will abort the entire operation if any commit fails to be submitted. This means that if `arc diff` succeeds on one commit and then fails on its child, the entire operation is aborted. However, the first `arc diff` had side effects, so the user gets diffs uploaded to Phabricator that are not reflected locally. Instead, we should confirm any passing commits and abort after we get a failing commit. This commit updates the Phabricator forge to handle the error case better and not produce garbage commits on Phabricator.
@arxanas arxanas force-pushed the arxanas/phabricator branch from 8c36f0b to 829a121 Compare August 9, 2024 01:37
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.

1 participant