Bunch of nitpicks #14728

emilio · 2016-12-25T17:35:26Z

I just noticed one while writing #14719, and then grepped and couldn't stop.

r? @nox

This change is

highfive · 2016-12-25T17:35:30Z

Heads up! This PR modifies the following files:

@bholley: components/style/thread_state.rs
@fitzgen: components/profile/time.rs, components/script/dom/eventdispatcher.rs, components/script/dom/element.rs, components/script/dom/node.rs, components/script/script_thread.rs, components/script/dom/domimplementation.rs
@KiChjang: components/script/dom/eventdispatcher.rs, components/script/dom/element.rs, components/net_traits/lib.rs, components/net_traits/lib.rs, components/script/dom/node.rs, components/script/script_thread.rs, components/script/dom/domimplementation.rs

highfive · 2016-12-25T17:35:31Z

Warning

These commits modify style, layout, gfx, and script code, but no tests are modified. Please consider adding a test!

wafflespeanut

Looks easy enough 😄

r=me after addressing the comments.

wafflespeanut · 2016-12-25T18:06:35Z

components/net_traits/lib.rs

-            None => self.headers = Some(Serde(Headers::new())),
-            Some(_) => (),
+        if self.headers.is_none() {
+            self.headers = Some(Serde(Headers::new()))


I've always used this, but I'm not sure whether we should prefer self.headers = self.headers.or(Some(Serde(Headers::new()))) over this.

Nit: Trailing semicolon here?

Looks more straight-forward and easy to read this way IMO.

wafflespeanut · 2016-12-25T18:06:35Z

components/profile/time.rs

-        }
-
-        self.buckets.insert(k, vec!(t));
+        self.buckets.entry(k).or_insert_with(Vec::new).push(t)


Nice! (semicolon please?)

Sure, if you feel strong about it... I don't personally, but fine

I don't really have an argument. Even though rustc doesn't care about these cases, I thought we should terminate expressions with semicolons, and leave it as optional for keywords. I'll better leave this to you :)

wafflespeanut · 2016-12-25T18:06:35Z

components/script/script_thread.rs

-                Some((size, size_type)) => resizes.push((id, size, size_type)),
-                None => ()
+            if let Some((size, size_type)) = document.window().steal_resize_event() {
+                resizes.push((id, size, size_type))


Nit: Trailing semicolon?

Same reply as above.


        script: Use more idiomatic code for resize handling.

Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>


        net: Simplify content_type setter.


        script: Simplify DOMImplementation::CreateHTMLDocument.


        script: Simplify propagation of disabled state for form controls.

That's a silly exponential algorithm btw.


        script: Simplify Node::collect_text_contents.


        script: Simplify ScriptMemoryFailsafe::drop.


        style: Simplify initialization of thread_state.


        gfx: Simplify OSX font template bytes accessor.


        layout: Simplify layout debugging scope.


        script: Simplify EventDispatcher's default action.


        profile: Simplify profiler's find or insert code.


        glutin: Simplify set_nested_event_loop_listener.

emilio · 2016-12-25T18:19:28Z

Addressed review comments, thanks for the review @wafflespeanut!

@bors-servo r=Wafflespeanut

bors-servo · 2016-12-25T18:19:29Z

📌 Commit f10e376 has been approved by Wafflespeanut

bors-servo · 2016-12-25T19:04:23Z

⌛ Testing commit f10e376 with merge 7fc9047...


        Auto merge of #14728 - emilio:nit, r=Wafflespeanut

@nox

Bunch of nitpicks I just noticed one while writing #14719, and then grepped and couldn't stop. r? @nox  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14728)

bors-servo · 2016-12-25T20:01:56Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev

highfive assigned nox Dec 25, 2016

highfive added the S-awaiting-review label Dec 25, 2016

wafflespeanut approved these changes Dec 25, 2016

View changes

emilio added 12 commits Dec 25, 2016

script: Use more idiomatic code for resize handling.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

755697d

Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>

net: Simplify content_type setter.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

d33ee45

script: Simplify DOMImplementation::CreateHTMLDocument.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

d1b175d

script: Simplify propagation of disabled state for form controls.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

157ce33

That's a silly exponential algorithm btw.

script: Simplify Node::collect_text_contents.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

2bbde04

script: Simplify ScriptMemoryFailsafe::drop.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

eaf69c2

style: Simplify initialization of thread_state.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

6264268

gfx: Simplify OSX font template bytes accessor.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

39e082a

layout: Simplify layout debugging scope.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

cd00d65

script: Simplify EventDispatcher's default action.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

cc262c6

profile: Simplify profiler's find or insert code.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

ed87fcf

glutin: Simplify set_nested_event_loop_listener.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

Loading status checks…

f10e376

emilio force-pushed the emilio:nit branch from dcffa07 to f10e376 Dec 25, 2016

highfive assigned wafflespeanut and unassigned nox Dec 25, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Dec 25, 2016

bors-servo merged commit f10e376 into servo:master Dec 25, 2016
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Dec 25, 2016

emilio deleted the emilio:nit branch Dec 27, 2016

servo / servo

Bunch of nitpicks #14728

Bunch of nitpicks #14728

emilio commented Dec 25, 2016 •

edited by larsbergstrom

highfive commented Dec 25, 2016

highfive commented Dec 25, 2016

wafflespeanut left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

emilio commented Dec 25, 2016

bors-servo commented Dec 25, 2016

bors-servo commented Dec 25, 2016

bors-servo commented Dec 25, 2016

servo / servo

Join GitHub today

Bunch of nitpicks #14728

Bunch of nitpicks #14728

Conversation

emilio commented Dec 25, 2016 • edited by larsbergstrom

highfive commented Dec 25, 2016

highfive commented Dec 25, 2016

wafflespeanut left a comment

This comment has been minimized.

wafflespeanut Dec 25, 2016

This comment has been minimized.

wafflespeanut Dec 25, 2016 • edited

This comment has been minimized.

emilio Dec 25, 2016

This comment has been minimized.

wafflespeanut Dec 25, 2016 • edited

This comment has been minimized.

emilio Dec 25, 2016

This comment has been minimized.

wafflespeanut Dec 25, 2016

This comment has been minimized.

wafflespeanut Dec 25, 2016

This comment has been minimized.

emilio Dec 25, 2016

emilio commented Dec 25, 2016

bors-servo commented Dec 25, 2016

bors-servo commented Dec 25, 2016

bors-servo commented Dec 25, 2016

emilio commented Dec 25, 2016 •

edited by larsbergstrom

wafflespeanut Dec 25, 2016 •

edited

wafflespeanut Dec 25, 2016 •

edited