Skip to content

Meeting 2015 11 02

Jump to bottom
Lars Bergstrom edited this page Nov 3, 2015 · 2 revisions

Servo Meeting 2015-11-02

Vidyo links:

Reminder: use http://benjamin.smedbergs.us/weekly-updates.fcgi/

Agenda items

Attending

  • larsberg, jdm, mbrubeck, ajeffrey, frewsxcv, manish, ms2ger, pcwalton, simonaspin, jack, edunham, bholley

Last week

Review carry over

  • jack: Brought up by bholley. Trying to get re-review has been holding him back. Idea is that people would have permission to do r=foo where foo previously reviewed the patch.
  • larsberg: Kinda makes me nervous - we've landed a bunch of missed bugs. I like manish's try support, though - that's awesome!
  • manish: Maybe just give them r+? Second option is adding a feature to homu that lets someone carry over a review if it was already r+'d. Not a fan of giving out global r+. On the fence about review carry feature.
  • jdm: Codifies existing practice. I usually don't look at rebases for r+s. I just take it as fact that those are safe.
  • pcwalton: Kinda agree with everybody. Maybe don't grant review-carry wildly, but for people who have done a few more patches we could consider it? Don't want to create a lot of process around it, but it would help.
  • bholley: Can you repeat?
  • mbrubeck: I'd be in favor of just giving r+. If we trust someone to review-carry, it's basically the same trust level. Probably don't need the technical solution - anyone we trust to review carry, we are giving them r+ and trust them not to misuse it.
  • larsberg: Hrm, I like it.
  • jdm: The review-carry that I propose just basically gives per-PR delegation of reviewer privileges, rather than a global level of extra privilege.
  • jack: Gecko policy?
  • jdm: Reviewers at any point can say "land now" or "make changes w/o re-review" and then it's delegated.
  • mbrubeck: Gecko has had no technical checking of reviews for commits.
  • bholley: On Gecko, there's a separation between being able to push to the repo from being a peer/module owner. Above that is peers & module owners. Helpful not to have extra-complicated policies with access checks. Once you're trusted to follow the rules and not upset people, then it
  • jack: Does anyone object in principle to implementing some solution for carry-over review?
  • [no objections]
  • jack: Okay, so we will implement this somehow, whether with Homu changes or process changes.

Dynamic testing

  • jack: Right now we run test-wpt and test-css on all changes, but coverage especially for dynamic/incremental layout changes are not very good.
  • bholley: Testing hover and activation is hard to do in automation. Gecko uses synthesized mouse events that Servo doesn't have. We can test most of the code paths by setting attributes, and I added some tests using that, though we still don't really test :hover.
  • jack: Does WPT have a plan to address this?
  • jdm: I think the plan is WebDriver, which has a cross-browser way to synthesize mouse events. (https://wiki.mozilla.org/Auto-tools/New_Contributor/Quarter_of_Contribution/Web_Driver_Infrastructure)
  • jack: But there are no current tests that use that?
  • jdm: Right. The plan is to take existing manual tests in the repository and rewrite them as webdriver tests.
  • jack: Are there manual tests that cover these?
  • jdm: Good question!
  • mbrubeck: Most of the Touch Events test suite is manual.
  • jack: If there are existing manual tests, we should run them.
  • bholley: If someone can point me to the manual tests, I can run through them.
  • jack: And in general we should have authors/reviewers do this for things that change dynamic layout until bors can run them.
  • bholley: Specifically things like hover/focus/active?
  • jack: That and anything that deals with mouse input or other things that the test harness can't deal with. We do have a WebDriver-based harness for Servo but we're still working on getting it running and with correct test expectations. We'll need to do that in order to use any automated tests that rely on WebDriver. Maybe we need to start filing issues for individual failures/differences and start investigating them.
  • bholley: Any plan to add Mochitest-compatible tests?
  • Manishearth: Isn't Mochitest for testing the browser chrome?
  • bholley: There are chrome- and content-flavored Mochitests. We have Mochitests for :hover selectors.
  • pcwalton: I'm fine with something like that, though I'd prefer not to get a dependency on Mochikit because it's old and unmaintained. It sounds like these tests aren't that dependent on the Mochikit framework.
  • jdm: Rewriting the tests would be a much better long-term plan.
  • jack: The other question for jgrahamc is what Gecko is going to do long-term, e.g. upstreaming these to WPT.
  • bholley: The problem is that in practice WPT still doesn't do a lot of the things Mochitest can.
  • jack: We should try to help them get there.
  • bholley: But meanwhile we are still writing new Mochitests, so the number of tests using Mochitest is growing, not shrinking.
  • jack: We wrote our reftest infrastructure with the plan of being compatible with existing Gecko reftests, but never got to the step of importing and running the Gecko tests. Instead we've now moved to WPT.
  • jack: Does anyone know if Mochikit works in Servo?
  • bholley: The Mochitest framework has diverged from Mochikit proper.
  • jack: Oh you people and your monorepos...

2016 Roadmap

  • jack: I had to prepare some goals for round 1/N of the annual goals process: https://github.com/servo/servo/wiki/Roadmap
  • jack: If anyone has things to add or change, please let me know. I have no idea about the deadline for this; probably before Orlando.
  • larsberg: Sooner is better, since we tend to get surprise deadlines sprung on us.
  • bholley: Is the Servo team working directly on oxidation (Rust in Gecko) or just supporting other people on it?
  • larsberg: Support mostly. The actual integration and landing has been done by people on the Gecko side so far.
  • larsberg: Editable internal spreadsheet: https://docs.google.com/spreadsheets/d/1HYoEo5Vx9XuFWFh_1zGWtT-pvebNqspY-PqbUzh3y7Q/edit#gid=0 Public read-only view: https://docs.google.com/spreadsheets/d/1HYoEo5Vx9XuFWFh_1zGWtT-pvebNqspY-PqbUzh3y7Q/pubhtml
  • jack: There are sort of two phases. One is the "little" oxidation tasks that larsberg has been tracking, just to get something in Rust riding the Gecko nightly train.. And then second, we need to figure out how to land a bigger component. It's not clear exactly what the deliverable will be, but it may include the stuff bholley is doing. This will be experimental, and our goal will be to support that, and we'll have people like Glenn and Patrick working on the WebRender side of it and collaborating on the style stuff.

autosquash

  • larsberg: Merge history right now is kind of weird, because each PR gets a merge commit. There are requests for automatic rebasing from homu rather than merge commits. People also said that would break tools/workflow, especially for things like bisect. No clear consensus, except that neither workflow is ideal. Two separate questions - do we rebase, and do we use --autosquash?
  • SimonSapin: Could we have autosquash on demand?
  • Manishearth: Not currently for homu. Might be able to fix it.
  • jack: autosquash would act like git's --autosquash; just squashes commits with fixup in the message.
  • mbrubeck: Don't see much use of fixup commits. Most of us squash and edit as we go, and many new git users are not aware of it.
  • jack: Easier to change commit messages than squash. Does anybody object to autosquash support? Sounds like everybody's fine with it, but it doesn't really address low hanging fruit.
  • Manishearth: Benefit is that you don't need a separate step to merge with fixup commits.
  • jack: I guess that I wouldn't put time into implementing autosquash support, but if it's there...
  • larsberg: Not sure if you can use autosquash without rebase enabled.
  • SimonSapin: It's possible to rebase --autosquash onto the existing common ancestor.
  • larsberg: Autosquash thumbs up, rebase thumbs down?
  • ms2ger: I approve of autosquash...

PR queue out of control

  • jdm: While not the worst it has ever been (91 open PRs), 85-ish is pretty bad. I'd appreciate more help looking for things they could review. I'd like to get this down. eefriedman has been helping with the DOM side of things, but more help would be great!
  • jack: I'll try to triage today.
  • jdm: If you are assigned to a review and it says S-awaiting-review, DO SO!
  • jack: A good problem to have!
  • bholley: People do seem really responsive, as a new person to the project.
  • jack: Bigger PRs bog us down. It's the little ones... but 60 of those 84 PRs will take substantial work.

Debug logging

  • mbrubeck: PSA! Logging at the debug level and below (debug! or trace!) is disabled on release builds. So, if you need the output, use error! or warn!. You can now be verbose with debug/trace and not affect release build performance. Also, if you want super-verbose, per-frame logging, use trace! so people can filter it out easily.
  • jdm: Where does info! fall?
  • mbrubeck: info is enabled everywhere. It's one level above debug.
  • bholley: Only when you enable logging? Or like printf?
  • mbrubeck: https://github.com/servo/servo/pull/8229
  • mbrubeck: Logs controlled by env variable. Difference is that now the debug and trace logs are statically disabled, compiled out, etc. The others can be disabled/enabled dynamically.

CSS 2.1 reftest

  • larsberg: gsnedders is working on porting the CSS 2.1 reftests, and as part of the CSS WG meetings he also kicked off the process of simplifying the upstreaming process and the build system. In the near future you'll be able to have normal reftests. He's starting that this week and will be working for the rest of the year on porting the remainder of those reftests.
  • Ms2ger: Assuming not too much pushback...
  • jack: I'll be glad not to have to deal with the old automation for that.
  • SimonSapin: The good news is that Ms2ger recently automated the building of those tests using Travis.
  • larsberg: All of the browser implementors at the CSS WG meeting said the build system was a barrier to upstreaming tests into that suite.

browswer.html

  • jack: Pushing on browser.html blocking bugs! If paul is commenting on your bugs, please answer them so he can get unblocked!
Clone this wiki locally