Inspect just last commit

[DavidGarciaCat]

When PUSH WebHook is sent to the WebHook URL then a batch of commits are queued. When we rebase a branch, all commits are queued and inspected again, one by one.

This is a waste of time and server resources, and PHPCI should analyse just the last commit (due it contains all updates included in that branch).

[JoolsMcFly]

One of those commits could break a build. If PHPCI skipped to the last commit it would report a broken build and you would think the last commit was the culprit when it is not. That could be misleading.

[DavidGarciaCat]

Hi @JoolsMcFly

Then maybe a Plan B could be skip a commit that was already analysed? Imagine a Pull Request has about 100 small commits, and then we rebase that Pull Request. When we push --force that rebased branch then we will queue 100 commits again, when we already inspected them and - more important - when we rebased from another branch that is already inspected too.

What do you think?

[JoolsMcFly]

I hear you. I've actually tweaked PHPCI for our needs and added a branch whitelist so it only builds master and our SprintN branches. Saves resources and build time when devs are working in their own branches.

May I ask why you have to use rebase? I have never used rebase myself, I always merge.

[DavidGarciaCat]

Given this case, how can I add this branch whitelist? Is there a way via phpci.yml configuration file to set-up a set of branches that I want to inspect and then ignore other branches?

Some times is easier for us to use rebase, specially when some of us are working with some code that changes every few minutes due all new updates (so we pull the parent branch and then rebase from there) - Probably you already know about this, just in case https://www.atlassian.com/git/tutorials/merging-vs-rebasing

[JoolsMcFly]

I just made a Q&D tweak to PHPCI/Controller/WebhookController.php's bitbucketWebhook method:

        foreach ($payload['push']['changes'] as $commit) {
            try {
                $email = $commit['new']['target']['author']['raw'];
                $email = substr($email, 0, strpos($email, '>'));
                $email = substr($email, strpos($email, '<') + 1);

                // I added this IF statement:
                if (preg_match('#^(?:Sprint_[0-9]*_Release|master)$#', $commit['new']['name'])) {
                    $results[$commit['new']['target']['hash']] = $this->createBuild(
                        $project,
                        $commit['new']['target']['hash'],
                        $commit['new']['name'],
                        $email,
                        $commit['new']['target']['message']
                    );
                }
                // patch ends here
                $status = 'ok';
            } catch (Exception $ex) {
                $results[$commit['new']['target']['hash']] = array('status' => 'failed', 'error' => $ex->getMessage());
            }

As you can see it's very straightforward, I just run a regexp on the branch name and voilà.

Thanks for your link. I actually checked this article after I pasted my message! I might start rebasing more often now. Thanks.

[DavidGarciaCat]

Another nice read, indeed.

👍 I will try to implement a similar update on my Webhook Controller, thanks!

[Gummibeer]

wouldn't it be good to add it as a general feature instead of two devs developing the same just for her self?

[DavidGarciaCat]

I think the real problem is all wasted time pulling and checking every single commit. I might know about a broken commit that is fixed on the next one and there is no sense to inspect what is not the latest change. All tools like Travis or Scrutinizer or SensioLabs Insight are inspecting just latest commits and their service just work.

[Gummibeer]

That's right - they inspect the last commit of one push. But if you push every single commit and/or push a fix for the breaking change they still inspect 2 commits. But I agree that it's a massive overload/waste of time to inspect every single commit in one push. And the "detect where the error occurred" isn't this right. I collect 5 commits, push them all at once: 1) fine 2) broken 3) fix 4) fine 5) fine

So I will get notified that one commit breaks my application even if it is fixed the second after and hasn't break my application at any time cause I've pushed it with my fix at once.

Next case: 1) broken 2) fix 3) fine 4) broken 5) broken

So I will get 3 broken notifications even if there is just one bug in my app, and the badest one - I will have to wait for the final result of my current app for 5 builds instead of 1. I can take a look in the log and will see which file has the error. And even if I push commits of others (yes it's possible) I'm the pusher and have to handle it cause I've bring in the bug.

Now take a real bad case - I've a lot of times - I push over 20/50/100 commits at once. If I run CS fixer I commit every single file to make it easier to revert it. Or I develop a big new feature, I commit in small parts but wait to push until I'm done. In this case I will have to wait for hours until I know if the current state is broken or fine.

I could think about users that want it that way - so I recommend a simple configuration option for it.