Thread overview
[dmd-internals] process / tool issues
Jun 06, 2017
Brad Roberts
Jun 07, 2017
Sebastian Wilzbach
June 06, 2017
So, what's missing from the current tooling and processes that has allowed these two scenarios:

1) 3 pull requests to master marked for auto-merge for weeks to not be merged:

    https://github.com/dlang/druntime/pull/1679 - 23 days
        broken on continuous-integration/jenkins/pr-merge

    https://github.com/dlang/druntime/pull/1732 - 30 days
        broken on continuous-integration/jenkins/pr-merge

    https://github.com/dlang/druntime/pull/1831 - 9 days
        waiting on continuous-integration/jenkins/pr-merge

2) 7 pull requests to stable to sit for days/weeks/months?  Pulls to stable should definitionally be simple low risk changes.  The oldest one is from August 2016.

I'm starting this thread in hopes that improvements can be made. Minimal time should be spent on how we got to that state or if it's anyone's fault.
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
June 06, 2017
> On Jun 6, 2017, at 6:48 PM, Brad Roberts via dmd-internals <dmd-internals@puremagic.com> wrote:
> 
> So, what's missing from the current tooling and processes that has allowed these two scenarios:
> 
> 1) 3 pull requests to master marked for auto-merge for weeks to not be merged:
> 
>    https://github.com/dlang/druntime/pull/1679 - 23 days
>        broken on continuous-integration/jenkins/pr-merge

It appears to have broken something in vibe.d

> 
>    https://github.com/dlang/druntime/pull/1732 - 30 days
>        broken on continuous-integration/jenkins/pr-merge

This one needs to be rebased. I had to do this to a PR from the hackathon that Sebastian told me to rebase, and it worked after the rebase.

> 
>    https://github.com/dlang/druntime/pull/1831 - 9 days
>        waiting on continuous-integration/jenkins/pr-merge

This one should have started testing by now, not sure.

> 
> 2) 7 pull requests to stable to sit for days/weeks/months?  Pulls to stable should definitionally be simple low risk changes.  The oldest one is from August 2016.
> 
> I'm starting this thread in hopes that improvements can be made. Minimal time should be spent on how we got to that state or if it's anyone's fault.

I think we need to seriously consider whether we should have the jenkins status required. There are too many dependencies, and I don’t even know if the build is redone properly if the dependent project has been updated and fixed a bug?

I understand the point, and I agree with the idea of it. But holding up progress with a test that hasn’t been completely fleshed out and clearly has bugs in it doesn’t help the situation.

I wonder if possibly some agent can email periodically this list of “stuck” PRs so we can pay attention. As I have mentioned before, the label being added isn’t sent in email, so it’s not obvious from just my email list that these things are supposed to be merged. I had no idea some of these were stuck for so long…

-Steve
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
June 07, 2017
Thanks a lot for noticing this & opening the discussion!

On 2017-06-07 04:50, Steven Schveighoffer via dmd-internals wrote:
>> On Jun 6, 2017, at 6:48 PM, Brad Roberts via dmd-internals <dmd-internals@puremagic.com> wrote:
>> 
>> So, what's missing from the current tooling and processes that has allowed these two scenarios:
>> 
>> 1) 3 pull requests to master marked for auto-merge for weeks to not be merged:
>> 
>>    https://github.com/dlang/druntime/pull/1679 - 23 days
>>        broken on continuous-integration/jenkins/pr-merge
> 
> It appears to have broken something in vibe.d

There were quite a few regressions with vibe.d since 2.074.0 that went undetected (see e.g. https://github.com/dlang-tour/dlang-tour/pull/519#issuecomment-304724314).
However, I think this one is just a transient issue (it just adds a test after all).

>> 
>>    https://github.com/dlang/druntime/pull/1732 - 30 days
>>        broken on continuous-integration/jenkins/pr-merge
> 
> This one needs to be rebased. I had to do this to a PR from the
> hackathon that Sebastian told me to rebase, and it worked after the
> rebase.

Yeah. Unfortunately has the Jenkins a lot of transient / unrelated failures and there's no easy way (yet) around this.
I think Martin wanted to give more people access to the server and/or implement auto-restart of failed builds from time to time.

>> 
>>    https://github.com/dlang/druntime/pull/1831 - 9 days
>>        waiting on continuous-integration/jenkins/pr-merge
> 
> This one should have started testing by now, not sure.

I just disabled the requirement for Jenkins to pass for the druntime repo (due to the instability it we also disabled it on the dmd and Phobos repo recently).
These PRs should get merged quite soon. One is already in.

>> 
>> 2) 7 pull requests to stable to sit for days/weeks/months?  Pulls to stable should definitionally be simple low risk changes.  The oldest one is from August 2016.
>> 
>> I'm starting this thread in hopes that improvements can be made. Minimal time should be spent on how we got to that state or if it's anyone's fault.
> 
> I think we need to seriously consider whether we should have the
> jenkins status required. There are too many dependencies, and I don’t
> even know if the build is redone properly if the dependent project has
> been updated and fixed a bug?

In theory the versions are locked, so that (without transient issues) red means there's a regressions.
IIRC especially getting packages from the DUB server tends to have random failures "regularly".
So, as mentioned above there are still some stability issues that need to be fleshed out with our ecosystem / DUB / Jenkins.
AFAIK there's also an unaddressed regression that slipped through during the time Jenkins was broken and disabled for dmd/phobos:

https://issues.dlang.org/show_bug.cgi?id=17472

Obviously if Jenkins is stable enough, the plan is to enforce its passing state, s.t. no regressions can silently slip through.

> I understand the point, and I agree with the idea of it. But holding
> up progress with a test that hasn’t been completely fleshed out and
> clearly has bugs in it doesn’t help the situation.
> 
> I wonder if possibly some agent can email periodically this list of
> “stuck” PRs so we can pay attention. As I have mentioned before, the
> label being added isn’t sent in email, so it’s not obvious from just
> my email list that these things are supposed to be merged. I had no
> idea some of these were stuck for so long…

The bot can already do this, it's just not deployed (yet) and instead of sending out mails,
for now (testing stage) it would just label such PRs, see for more information this PR:

https://github.com/dlang-bots/dlang-bot/pull/52

> -Steve
> _______________________________________________
> dmd-internals mailing list
> dmd-internals@puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals