Problem: People rubber stamp or TBR rebaselines instead of doing normal reviews, because they are hard to review, due to lack of detailed knowledge about why they are being rebaselined. This is causing bad baselines to be checked in, which leads to layout test failures, which leads to sadness. Proposed Solution: Changelist comments for rebaselines should include enough information that the reviewer should be able to do a reasonable sanity check (aka, explain a little about WHY this is the right thing to be doing in the first place), and reviewers should do a quick sanity check. Spending a couple seconds glancing at diffs now will save time in the future.
Real examples I ran into in the past 2 days: - Failing tests because the baseline checked in is for an error page, and we generate real results. Glancing at the diff should have caught this: http://codereview.chromium.org/113170/diff/1/1080 - A slew of totally unrelated tests had one single image checked in as their baseline. Again, glancing at the diff should have caught this: http://codereview.chromium.org/99147/diff/1/12 Julie --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
