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 list rebased commits; Detect a force push #68

Open
Viech opened this issue Apr 7, 2014 · 3 comments
Open

Don't list rebased commits; Detect a force push #68

Viech opened this issue Apr 7, 2014 · 3 comments

Comments

@Viech
Copy link

Viech commented Apr 7, 2014

If commits of a branch are rebased on top of another branch, these commits get a new commit hash when the updated branch gets force pushed, which makes the bot assume that they are new commits (and list them).

As an example scenario, there is a topic branch based off master. While the topic branch is being worked on by a single developer, the developer finds bugs on the master branch that need to be fixed to make the topic branch work. However, putting these fixes in the topic branch would mean that their introduction into master can be delayed quite a bit. So the developer decides to fix the bugs on the master branch and pull them into the topic branch via git rebase master (so that the topic branch isn't filled with empty merge commits). This will rewrite the history of all the topic commits. When the topic branch is now pushed (the force switch is needed), the bot lists the topic commits as if they were entirely new, which can create a wrong impression and leads to spam when done frequently. Furthermore, no indication is given that the branch has been force-pushed.

The bot should warn whenever existing commits are removed (any force push) and in case a commit has been relabeled with a new commit hash (every rebase), detect that it's really the same commit under a new name.

@TkTech TkTech added irc and removed irc labels Apr 8, 2014
@TkTech
Copy link
Owner

TkTech commented Apr 8, 2014

Is this for Github? Bitbucket?

If this is for Github, the bot relies on the "distinct" field on each commit in a push, which is provided by Github. Notifico does not (and we probably don't want to) store every commit. That would immediately bloat the database by over a million rows.

I can't think of any method for determining if a push is a force push at the moment.

@Viech
Copy link
Author

Viech commented Apr 8, 2014

This is for GitHub.

A force push should be relatively easy to detect by keeping track of the head commit. If "new" commits are pushed and the old head commit isn't directly below the sum of all pushed commits afterwards (or in other words it isn't among their parents), you can assume that it has been removed or relabeled, hence force had to be used to push those new commits. Since the former head commit would also get a new hash if one of its parents was edited, you shouldn't miss a force push that way either.

Detecting relabeled commits is certainly harder. Ideally one would need to hash the commit content as such but if such a checksum is not provided by the GitHub API it's understandable that you don't want to download the actual commit contents to hash them.

@ghost
Copy link

ghost commented Aug 6, 2015

No idea if that was true back when this issue was opened, but the GitHub API now sends {"forced": true} for force pushes. Keep an eye out for {"created": true} though, as creating a new branch seems to always count as a force-push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants