On 6/22/16 11:17 AM, Manish Goregaokar wrote:
We don't close down the tree except for CI fires
Yes, I understand your model. You don't have to explain it to me.
I was just pointing out that for normal commits you prefer the model of
test-each-thing, but for style system you would prefer the model of
just-test-every-so-often. I realize the reasons for that too, but I
think we have a different evaluation of the resulting costs. In
particular....
If the tests fail for a PR, it is taken out of the queue until it is fixed and
reapproved.
My question is what happens when a PR stalls like this for months
because by the time it's fixed something else has broken. I expect this
to happen every so often when the PR is the style system merge.
m-c does not have this model, and I'd also like to prevent marrying our CI
times to those of m-c, or being affected by m-c backouts.
I understand and sympathize with that.
That's why I like the sync model, it concentrates all of the conflict in one
operation
At the cost of possibly making that operation impossible to complete.
instead of distributing it everywhere and making every PR that touches
anything that affects style suffer. The syncing operation is also async
(ha!) -- while being performed other Servo work can continue.
Which is what may well make it impossible to complete the sync.
Sending PRs which look like they need testing to gecko-try should catch most
issues,
the only remaining issues are when a PR (like a wrong refactor, or
something) which wasn't sent through gecko-try breaks stylo, or when
something on Gecko's vendor dir managed to land between the time the Servo
PR was tested and landed. I postulate these two cases will be relatively
rare, and these will be the only times the sync fails.
Right, I think our fundamental disagreement is a matter of assumptions
about frequency. I hope that you're right about these failures being
rare, but expect that you're wrong and worry about the failure modes if
you are....
This is actually pretty close to the proposed model in the doc, but it
gives a lot more leeway in when the sync should happen.
Well, and assumes that reviewers are decent at evaluating the "may need
stylo testing" predicate.
The proposed model is basically equivalent to this except syncs are mandatorily
part of any PR
that affects style, and can make the queue clog up. Here based on various
factors you can choose not to sync (e.g. if the queue is already full; you
can wait to trigger the sync until it empties, or if the PR probably
doesn't need a sync).
I agree that having such flexibility is not unreasonable.
We can then by trial and error figure out the best
policy for when to sync and when not to sync balancing the need for fast CI
times and the need for things to not cause colossal merge conflicts.
That seems fair. Ideally we would end up with a "sync pretty much every
time" setup.
Sure they can, but they should be less likely to :)
That hasn't been my experience with refactorings at all!
We will still have the full CI run on a sync so these will get caught, just
not immediately.
Right. The question is how much piles up before a sync. So more
granular syncs are better if they can be arranged.
-Boris
_______________________________________________
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo
