On 5/23/18 4:51 PM, Manu wrote:
I'm not suggesting adding new machines... I'm suggesting removing the
ones that take ~50 minutes.
I think this thought has merit. Or at least delegate them to only test
older PRs.
I guess the flow I observed on the weekend, where it took me 3 days to
get my batch of PR's merged, is that people reviewed it once it was
already green... so that's at least 50 minutes at the end of my actual
work, THEN they ask me to add `package` or `const` somewhere, so I do,
I don't think everyone does this, and I'd be surprised at any who do,
with the caveat that if a PR is for a specific platform, they may wait
to ensure the PR passes that platform before approving.
My workflow is to FIRST look at the PR and see if it looks good, then if
needed, take a look at the auto tester. Hell, I often times approve a PR
with an obvious syntax error that I didn't notice, only to see the auto
tester fail later.
So it was probably a coincidence that they asked for the changes after
the tests were complete.
and that's another hour before they have another look to confirm the
tweak, and then they probably got bored of reviewing PR's in the last
2-3 hours, and went to bed, or to work, or to get lunch or something,
so they reappear the next day.
So in practise, adding 2-3 hours to the cycles adds 24 hours to the
cycle. The latency was the problem for all of my 5-6 patches, not the
throughput.
I'm going to level with you, not everyone is on Manu's schedule to get
things done this weekend. Sorry. This is open-source/volunteer workflow.
I would be more likely to believe it's just people are going on their
own schedule when they can donate their time. I submitted a PR at the
hackathon, and it was approved as I was flying home. Still not merged
(2+ weeks later). I don't think it has anything to do with the tester.
If these were DMD PRs, you are in for latency that has nothing to do
with test machines -- there just aren't a lot of people who are
qualified to or feel comfortable with reviewing PRs.
And ping people who have reviewed, or who you think might be inclined to
merge. Sqeaky wheels and all that.
Maybe those 4 machines could be assigned a rule, where they're only
assigned jobs to re-test PR's with updated DMD's or whatever if the PR
has been silent for >24 hours or something... so they are assigned to
low-frequency jobs, and never assigned to fresh jobs?
Yes, I think this idea makes sense. It's really up to Brad though.
-Steve
