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

Don't add 'rebased-with-history from <commit>' if IMERGE_MODIFY_COMMI… #134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brucehoult
Copy link

@brucehoult brucehoult commented Oct 8, 2018

After running rebase-with-history (which is otherwise great) many times, the commit messages get horrible.

I think it's better to not modify the messages. Enable this to be opted-out of globally by setting an environment variable.

You can see the hashes of all the parents of the commit in the git log for it anyway.

@@ -2831,6 +2831,15 @@ class MergeState(Block):
)
self.git.checkout(refname, quiet=True)

def get_modified_log_message(self, orig, kind):
msg = self.git.get_log_message(orig)
if os.environ.get('IMERGE_MODIFY_COMMIT_MSG','1') != '0':
Copy link
Author

Choose a reason for hiding this comment

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

No point to cache/optimise this as the subsequent git operation is far more heavyweight.

@mhagger
Copy link
Owner

mhagger commented Oct 12, 2018

I like the idea, but why an environment variable? Wouldn't it be more natural to use a git configuration setting, as we already do for imerge.editMergeMessages? That would also allow users to configure it globally or per repo.

Another question: would it be useful to add the rebased with history from <commit> line, but at the same time delete any previous such comments in the log message? That would give readers the information that rebase-with-history was involved and give them a pointer to the previous version of the commit, but not allow the messages to accumulate forever.

@brucehoult
Copy link
Author

Ok sure git config would work.

Replacing any previous "rebased with history" (etc) line would be a better option than the current default, but I still personally prefer nothing at all.

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.

2 participants