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

Remove breaks when used with pull #492

Open
krokofant opened this issue Apr 3, 2023 · 2 comments
Open

Remove breaks when used with pull #492

krokofant opened this issue Apr 3, 2023 · 2 comments
Labels
status: pinned Should not be labeled as stale

Comments

@krokofant
Copy link

krokofant commented Apr 3, 2023

Consider this:

- uses: EndBug/add-and-commit@v9
  with:
    add: '["file-that-changed.txt"]'
    remove: '["file-that-was-removed.txt"]'
    author_name: GitHub-Automated
    author_email: foo@example.com
    default_author: github_actor
    message: "Changing files"
    pathspec_error_handling: exitImmediately
    pull: "--rebase --autostash"
    push: false

First the file is removed here

Then if pull is used, the file is also removed here which breaks the action, since the file is already marked as removed.

::error::Error: fatal: pathspec 'file-that-was-removed.txt' did not match any files
::error::Error: Remove command did not match any file:%0A git rm file-that-was-removed.txt

If one wants to rebase it does not make sense that the commit is done after the pull. The commit should be done, then a pull rebase should happen, that would not break the flow.
I guess it's hard to account for these different approaches.

@krokofant krokofant added the status: pending More info is needed before deciding what to do label Apr 3, 2023
@EndBug
Copy link
Owner

EndBug commented Apr 3, 2023

You're probably right, but honestly I'm afraid that if I change that behavior then I'm going to break someone else's workflow :/
It's not destructive, since I'd use a new major version, but it would mean that then someone else could open an issue like this...
I don't know, do you think there's a way to make everyone happy about this? 😅

@krokofant
Copy link
Author

krokofant commented Apr 4, 2023

😅 I don't think a workaround for this specific situation is good. I think it's reasonable to expect that a commit needs to be done before another pull is done.

If people use this action to say "ignore everything that has happened in origin, I just really want these files removed, then the workflow is flawed.

Currently a user can:

  1. Remove all *.md files
  2. Only README.md was removed
  3. git pull => now there is a IMPORTANT-NOTE.md
  4. Remove all *.md files
  5. Now the README.md and the IMPORTANT-NOTE.md is removed

I think it's reasonable to encourage the user to fix the conflict somehow, and not just hide it. If the commit is done just after the first round of add/remove, then those changes are clearly described. Then follows a potential pull or pull --rebase which describes where in the history they'd want their commit. If there are any conflicts between your commit and any new commits in origin, you will be notified of the conflict since the action will fail.

@EndBug EndBug added status: pinned Should not be labeled as stale and removed status: pending More info is needed before deciding what to do labels Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: pinned Should not be labeled as stale
Projects
None yet
Development

No branches or pull requests

2 participants