Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:73822
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.128.170 as permitted sender)
MIME-Version: 1.0
Sender: julienpauli@gmail.com
In-Reply-To: <535D5E07.4060607@sugarcrm.com>
References: <535A13E1.3050106@sugarcrm.com> <alpine.DEB.2.10.1404261028260.4404@whisky.home.derickrethans.nl>
 <535D5E07.4060607@sugarcrm.com>
Date: Mon, 28 Apr 2014 11:05:17 +0200
Message-ID: <CAMUwpuS5yrJGei=n74pP7KJppAveJGb_5XwDe-L3pOFeUAyZ6w@mail.gmail.com>
To: Stas Malyshev <smalyshev@sugarcrm.com>
Cc: Derick Rethans <derick@php.net>, PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [VOTE] CI tests RFC
From: jpauli@php.net (Julien Pauli)

On Sun, Apr 27, 2014 at 9:44 PM, Stas Malyshev <smalyshev@sugarcrm.com> wrote:
> Hi!
>
>> I have a comment (not specifically to you). We can't seriously be
>> suggesting that RMs can just revert commits. This is a really rude thing
>> to do in an open source project. We're doing this for fun, and people
>> immediately reverting your commits takes (more) fun out of it.
>
> I agree. That's why we have time limits before that should happen.
> However, it has happened that people made commits which break things and
> then have gone unresponsive for an extended period of time, with broken
> commits laying there and making everybody work much harder to ensure
> breakage does not spread (if CI is red, then any pull against it is red,
> so we can't really trust the pulls tests, and when we merge them we
> don't know anymore what exactly broke it and it becomes a mess very
> quickly). So I think the normal workflow would be as follows:
>
> 1. Make pull
> 2. See the pull test green
> 3. Merge the pull
> 4. Ensure the CI for main branch is still green
> 5. Go have a beer/coffee/well-deserved rest

No offense, but I've kept thinking it was the case till now.

Hang on, talking about my own 5.5 RM experience, and knowing we are
talking about an open source project, I always privilege the
discussion when there is some conflict.
It has happened that I had failing tests in a release branch, then I
go thought git history and try to contact the guy behind this.
If no answer, there is no other choice than reverting (or trying to
fix the case, which often is not an option as it may be hard).

I agree that the default case should always be to have a green PR. In
fact, I cant understand someone trying to PR something turning tests
to red, that's just over my mind, but if you say that happens, OK so.

I would tend to say that if you red the tests, you'll never be merged.
Someone still can contact the RM in case of any problem, but guy, if
you put tests to red, if you think your work is not finished yet
(weither you have time to finish it or not) : just don't ask for PR.
The only thing it will bring is confusion to other contributors, and
RM fighting with your commits and probably with you as well :-P

I don't really like exceptions. If you turn the tests to red and say
you have no time for now to green them back, then why not just wait
nicely in your feature branch, and we'll merge it later ? Why rush ?

Git is wonderful at collaboration, there is no need to rush to force a
PR to be merged knowing it will fail tests and introduce some bad code
in a stable branch, that is just not acceptable.

Regarding XFails, I have no advice about them. I dont really know why
they are here etc...

Having said that, I agree with Ferenc saying we have very uncommon
situations where some tests fail for stable releases, which is a very
good point.

Julien