Meeting 2013 12 16

Agenda

• Critic review tool (jdm)
• Rust upgrade in progress
  • unused code style?
  • use do spawn {} or spawn(proc() {})?

Critic review tool

• jack: jdm & jgraham recommended a different review tool (developed by Opera). Part of a "is github meeting our needs" question. External tools have things like side-by-side commits, etc. And not losing review history when we squash/rebase commits.
• dherman: Nothing that would discourage contributors in the process.
• jack: Critic used in normal github workflow as an optional tool. PRs on github. Reviewers can just use github and it's the same process/login/etc. There's a bot posting critic review URLs to current PRs. Everybody try it for next review? I noticed the repo tool (to replace submodules), which is what FFOS & Android use, has integration with their tool (garrit). Maybe repurpose that to do github stuff as well? Or upload to critic?
• larsberg: Just concerned about increasing our cost from zero w/ github. brson?
• brson: No significant complaints. Rust is not planning to change things.
• dherman: github's where all the energy/momentum is. It's a strong bet to make, servo is in for the long haul, github will be improving. We could talk to them about particular pain points. Stopgaps OK. And optional is fine. But investing in something else and leaving us stuck there without being able to move back would be bad.

Rust upgrade

• jack: going pretty smoothly. Now, there's a dead code warning! Some is not obviously dead. e.g., in style crate, a few are reported unused. Maybe they were meant to be public? Or were planned to be used in the future? For now, like in opengles, I just comment them out. Seems to keep them in a good place.
• brson: Commenting out seems like a bad sign...
• jack: Maybe wait for Simon?
• brson: Disable per-module with allow?
• kmc: If rust-opengles has constants, it should be exporting them, right? And not be dead?
• jack: core-foundation has tons of internal statics in the headers that might be used later?
• kmc: I'd just disable the lint for those modules.
• jack: I just commented them out. Somebody already did the work to port it.
• larsberg: Just worried because Rust cost atrohpies if commented.
• jack: Will talk to Simon, since mainly in style crate.
• jack: Other, is do going to survive at all? I can use spawn as spawn(proc instead of do spawn.
• kmc: do notation is becoming specialized for do spawn, so maybe we should try to leave it as that.
• jack: Gist is: "what would brian do?"
• brson: do is doomed.
• jack: spawn(proc it is. Then, less changes to make later.
• jack: Also, Cell is gone, which is good. Except, sometimes we need to clone things before they go into the closure, and there's no way to do it except doing let url_clone = url.clone(). Will there be something in the future to make that less ugly?
• brson: use spawn_with
• kmc: Doesn't clone. But we could write a macro that does. pcwalton was talking about C++-style capture specifiers and capture-by-clone. but don't know if it's coming.
• jack: That sounds like a really big change.
• kmc: Probably have to write a spawn_with macro. But only want to clone some things.
• jack: You mean we have our own macro?
• kmc: Yes. Have our own spawn_with macro that calls it. The macro calls spawn_with with a tuple...
• jack: rust deleted task::spawn_with
• kmc: spawn_with and Cell both went away?
• jack: Yes. In current master, there's no spawn_with. It has been deleted. All of the replacements were very straightforward. Maybe I'll play with some macros to make it nicer. I'm currently in net dealing with the downloading callbacks.

Other items?

• kmc: Getting linked task failure stuff running soon. Just hard to test that we're actually pulling them down. But usually they die when they try_recv on a closed port.
• brson: Now senders without a connection also die.
• jack: Also, a PR is incoming for "i tried" since it breaks the test since it tears down the javascript backend too quickly. Next on my agenda after rust upgrade is getting testing turned back on. Ref tests, contents tests at least on Mac, and linux with larsberg.
• larsberg: Have nvidia running again. Will commit it. Some long-term surface questions remain (particularly around out of process), but we'll table those for now.