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.

Sign up for GitHub

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

Jump to bottom

fix: enable changing commit message when using amend #3954

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix: enable changing commit message when using amend #3954

wants to merge 2 commits into from

Conversation

amorscher
Copy link
Contributor

@amorscher amorscher commented Feb 6, 2024 •
edited

allows overriding of the commit message

Motivation and Context

fixes #3936

How Has This Been Tested?

all tests still pass

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (change that has absolutely no effect on users)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented Feb 6, 2024 •
edited

☁️ Nx Cloud Report

CI is running/has finished running commands for commit dedb1f0. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

🟥 Failed Commands
nx run-many -t test --parallel=3 --ci --maxWorkers=2
nx run-many -t test --parallel=3 --ci --maxWorkers=2 --testTimeout=60000 --testPathIgnorePatterns=save.spec.ts --testPathIgnorePatterns=find-file.spec.ts --testPathIgnorePatterns=chain-class.spec.ts
✅ Successfully ran 5 targets

Sent with 💌 from NxCloud.

@amorscher amorscher marked this pull request as draft February 6, 2024 20:09
@amorscher
Copy link
Contributor Author

@JamesHenry I changed the behaviour of --amend in combination with --message so that the message will be overrridden if specified. I think this is a breaking change and therefore not a good solution.

What do you think about using another flag like --amendMessage to support amending and overriding the message in one go?

@amorscher
Copy link
Contributor Author

amorscher commented Feb 11, 2024 •
edited

@fahslaj what do you think?

@fahslaj
Copy link
Contributor

fahslaj commented Feb 27, 2024

Hey @amorscher , I think it's fine to update --amend and --message to behave as you stated at first, where passing them both together will properly override the message. It would technically be a breaking change, but I think that this is really a bug fix and that most users would expect --amend and --message to work together anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Using "--amend" and "--message" options will not overwrite the original commit message
2 participants
@amorscher @fahslaj