Skip to content
This repository was archived by the owner on Apr 12, 2019. It is now read-only.

Prefix commit ID in git log pretty to ease detection #71

Closed
wants to merge 1 commit into from
Closed

Prefix commit ID in git log pretty to ease detection #71

wants to merge 1 commit into from

Conversation

lafriks
Copy link
Member

@lafriks lafriks commented Jun 26, 2017

@lafriks
Copy link
Member Author

lafriks commented Jun 27, 2017

@ethantkoenig

@ethantkoenig
Copy link
Member

@lafriks Unfortunately, this won't work. What if there is an entry that has only been affected by merge commits? If we ignore merge commits, we won't be able to find a commit for that entry.

I realize there are some problems with the current implementation of GetCommitsInfo (see #72), and I'm working on fixing them ASAP.

@lafriks
Copy link
Member Author

lafriks commented Jun 27, 2017

@ethantkoenig How do you get merge commit with entries? I thought merge commit allways has child commit

@ethantkoenig
Copy link
Member

@lafriks You can git commit --amend a merge commit, so a merge commit can contain arbitrary changes.

@lafriks
Copy link
Member Author

lafriks commented Jun 27, 2017

I thought amend allows changing just commit message but OK, I will rework to ignore hashes with no entries

@ethantkoenig
Copy link
Member

@lafriks In light of #72, all of this code will probably need to be rewritten (or at least heavily refactored), so I wouldn't bother (unless you really want to).

@ethantkoenig
Copy link
Member

@lafriks I think we should revert to the old implementation of GetCommitsInfo(..) for the 1.2.0 release (see #73), so please close this issue.

@lafriks
Copy link
Member Author

lafriks commented Jun 27, 2017

@ethantkoenig I have changed pretty format for git log command so that commit IDs would be prefixed and can be easily recognized

@lafriks lafriks changed the title Skip merge commits in log command when listing directory entries Prefix commit ID in git log pretty to ease detection Jun 27, 2017
@ethantkoenig
Copy link
Member

This does fix go-gitea/gitea#2064, but it doesn't fix a lot of other known issues:

so I still think reverting (#73) is the best move right now.

@lafriks lafriks closed this Jun 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants