Convert imports in generate_imports into one per line, so that it can… #13070

tp6vup54 · 2016-08-27T04:09:11Z

Convert imports in generate_imports into one per line, so that it can easily add new ones with alphabetical order.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #13036 (github issue number if applicable).

There are tests for these changes OR
These changes do not require tests because _____

… easily add new ones with alphabetical order.

This change is

KiChjang · 2016-08-27T04:10:06Z

components/script/dom/bindings/codegen/CodegenRust.py

        'dom::types::*',
        'js::error::throw_type_error',
        'js::jsapi::JSContext',
-        'js::jsapi::{HandleValue, MutableHandleValue}',
+        'js::jsapi::HandleValue',


Looks like this line should be above the previous line.

KiChjang · 2016-08-27T04:11:49Z

components/script/dom/bindings/codegen/CodegenRust.py

@@ -5321,95 +5324,216 @@ def generate_imports(config, cgthings, descriptors, callbacks=None, dictionaries

    return CGImports(cgthings, descriptors, callbacks, dictionaries, enums, [
        'js',
-        'js::{JS_CALLEE, JSCLASS_GLOBAL_SLOT_COUNT}',
-        'js::{JSCLASS_IS_DOMJSCLASS, JSCLASS_IS_GLOBAL, JSCLASS_RESERVED_SLOTS_MASK}',
+        'js::JS_CALLEE',


_ is greater than C, so this should be put after the js::JSCLASS_* declarations.

KiChjang · 2016-08-27T04:14:11Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'js::jsapi::JS_SetImmutablePrototype',
+        'js::jsapi::JS_SetProperty',
+        'js::jsapi::JS_SetReservedSlot',
+        'js::jsapi::JS_SplicePrototype',


Same problem as above - _ is greater than any Latin capital letters.

KiChjang · 2016-08-27T04:15:09Z

components/script/dom/bindings/codegen/CodegenRust.py

        'std::borrow::ToOwned',
        'std::cmp',
        'std::mem',
        'std::num',
        'std::os',
-        'std::panic::{self, AssertUnwindSafe}',
+        'std::panic::{self}',


Remove braces here.

If braces are removed here, there will be an error occurred.

error[E0429]:selfimports are only allowed within a { } list

Seems like the braces cannot be removed?

Ah, right, you need to remove ::{self} entirely, i.e. the final declaration would look like std::panic.

KiChjang · 2016-08-27T04:16:40Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'js::jsval::ObjectValue',
+        'js::jsval::ObjectOrNullValue',
+        'js::jsval::PrivateValue',
+        'js::jsval::NullValue',


N comes before O.

KiChjang · 2016-08-27T04:17:09Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'js::jsval::NullValue',
+        'js::jsval::UndefinedValue',
+        'js::glue::CallJitMethodOp',
+        'js::glue::CallJitGetterOp',


G comes before M.

KiChjang · 2016-08-27T04:18:04Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'js::glue::NewProxyObject',
+        'js::glue::ProxyTraps',
+        'js::glue::RUST_JSID_IS_STRING',
+        'js::glue::int_to_jsid',


Lowercase letters after uppercase.

KiChjang · 2016-08-27T04:19:18Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'dom::bindings::interface::define_guarded_properties',
+        'dom::bindings::interface::is_exposed_in',
+        'dom::bindings::interface::ConstantVal::IntVal',
+        'dom::bindings::interface::ConstantVal::UintVal',


Put before InterfaceConstructorBehavior.

KiChjang · 2016-08-27T04:20:10Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'dom::bindings::interface::ConstantVal::IntVal',
+        'dom::bindings::interface::ConstantVal::UintVal',
+        'dom::bindings::iterable::IteratorType',
+        'dom::bindings::iterable::Iterable',


Put above IteratorType.

KiChjang · 2016-08-27T04:20:27Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'dom::bindings::js::JS',
+        'dom::bindings::js::Root',
+        'dom::bindings::js::RootedReference',
+        'dom::bindings::js::OptionalRootedReference',


Put above Root.

KiChjang · 2016-08-27T04:22:38Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'dom::bindings::callback::CallbackContainer',
+        'dom::bindings::callback::CallbackInterface',
+        'dom::bindings::callback::CallbackFunction',
+        'dom::bindings::callback::CallSetup',


Put above CallbackContainer.

KiChjang · 2016-08-27T04:23:55Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'dom::bindings::conversions::ConversionResult',
+        'dom::bindings::conversions::DOM_OBJECT_SLOT',
+        'dom::bindings::conversions::IDLInterface',
+        'dom::bindings::conversions::is_array_like',


Put above jsid_to_str.

KiChjang · 2016-08-27T04:24:34Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'dom::bindings::conversions::ConversionBehavior',
+        'dom::bindings::conversions::ConversionResult',
+        'dom::bindings::conversions::DOM_OBJECT_SLOT',
+        'dom::bindings::conversions::IDLInterface',


Put above StringificationBehavior.

KiChjang · 2016-08-27T04:25:16Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'dom::bindings::conversions::native_from_handlevalue',
+        'dom::bindings::conversions::native_from_object',
+        'dom::bindings::conversions::private_from_object',
+        'dom::bindings::conversions::root_from_object',


Put after root_from_handlevalue.

KiChjang · 2016-08-27T04:25:39Z

components/script/dom/bindings/codegen/CodegenRust.py

+        'dom::bindings::codegen::PrototypeList',
+        'dom::bindings::codegen::RegisterBindings',
+        'dom::bindings::codegen::UnionTypes',
+        'dom::bindings::error::Fallible',


Put after ErrorResult.

KiChjang · 2016-08-27T04:26:28Z

components/script/dom/bindings/codegen/CodegenRust.py

        'dom::browsingcontext::BrowsingContext',
        'mem::heap_size_of_raw_self_and_children',
        'libc',
        'util::prefs::PREFS',
-        'script_runtime::{store_panic_result, maybe_take_panic_result}',
+        'script_runtime::store_panic_result',


Put after maybe_take_panic_result.

KiChjang · 2016-08-27T06:43:19Z

Looks great! All that's left now is to squash your commits into one.


        Convert imports in generate_imports into one per line, so that it can…

… easily add new ones with alphabetical order.

jdm · 2016-08-27T07:36:40Z

@bors-servo: r=KiChjang

bors-servo · 2016-08-27T07:36:41Z

📌 Commit 59dcb31 has been approved by KiChjang

bors-servo · 2016-08-27T09:08:30Z

⌛ Testing commit 59dcb31 with merge 85838c0...


        Auto merge of #13070 - tp6vup54:master, r=KiChjang

Convert imports in generate_imports into one per line, so that it can…  Convert imports in generate_imports into one per line, so that it can easily add new ones with alphabetical order. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #13036 (github issue number if applicable).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____  … easily add new ones with alphabetical order.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13070)

bors-servo · 2016-08-27T10:20:45Z

💔 Test failed - mac-rel-wpt

highfive · 2016-08-27T10:20:48Z

  ▶ TIMEOUT [expected PASS] /_mozilla/css/iframe/hide_layers2.html

KiChjang · 2016-08-27T10:22:44Z

@bors-servo retry

#13059

bors-servo · 2016-08-27T12:27:51Z

⌛ Testing commit 59dcb31 with merge 1fd2577...


        Auto merge of #13070 - tp6vup54:master, r=KiChjang

Convert imports in generate_imports into one per line, so that it can…  Convert imports in generate_imports into one per line, so that it can easily add new ones with alphabetical order. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #13036 (github issue number if applicable).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____  … easily add new ones with alphabetical order.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13070)

bors-servo · 2016-08-27T13:25:42Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev

KiChjang reviewed Aug 27, 2016
View changes

jdm added the S-awaiting-review label Aug 27, 2016

jdm assigned KiChjang Aug 27, 2016

KiChjang reviewed Aug 27, 2016
View changes

components/script/dom/bindings/codegen/CodegenRust.py Outdated

'js::jsval::ObjectValue',

'js::jsval::ObjectOrNullValue',

'js::jsval::PrivateValue',

'js::jsval::NullValue',

This comment has been minimized.

Sign in to view

Copy link

KiChjang Aug 27, 2016
Member

N comes before O.

KiChjang reviewed Aug 27, 2016
View changes

components/script/dom/bindings/codegen/CodegenRust.py Outdated

'js::jsval::NullValue',

'js::jsval::UndefinedValue',

'js::glue::CallJitMethodOp',

'js::glue::CallJitGetterOp',

This comment has been minimized.

Sign in to view

Copy link

KiChjang Aug 27, 2016
Member

G comes before M.

KiChjang reviewed Aug 27, 2016
View changes

KiChjang added S-needs-code-changes and removed S-awaiting-review labels Aug 27, 2016

highfive added S-awaiting-review and removed S-needs-code-changes labels Aug 27, 2016

KiChjang added S-needs-squash and removed S-awaiting-review labels Aug 27, 2016

Convert imports in generate_imports into one per line, so that it can…

Loading status checks…

59dcb31

… easily add new ones with alphabetical order.

tp6vup54 force-pushed the tp6vup54:master branch from 11a430a to 59dcb31 Aug 27, 2016

highfive added the S-awaiting-review label Aug 27, 2016

shinglyu modified the milestone: Taiwan Code Sprint Aug 27, 2016

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-squash labels Aug 27, 2016

highfive removed the S-awaiting-merge label Aug 27, 2016

highfive added the S-tests-failed label Aug 27, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Aug 27, 2016

bors-servo merged commit 59dcb31 into servo:master Aug 27, 2016
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

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

homu Test successful
Details

highfive removed the S-awaiting-merge label Aug 27, 2016

servo / servo

Convert imports in generate_imports into one per line, so that it can… #13070

Convert imports in generate_imports into one per line, so that it can… #13070

tp6vup54 commented Aug 27, 2016 •

edited by larsbergstrom

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.

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.

This comment has been minimized.

This comment has been minimized.

KiChjang commented Aug 27, 2016

jdm commented Aug 27, 2016

bors-servo commented Aug 27, 2016

bors-servo commented Aug 27, 2016

bors-servo commented Aug 27, 2016

highfive commented Aug 27, 2016

KiChjang commented Aug 27, 2016

bors-servo commented Aug 27, 2016

bors-servo commented Aug 27, 2016

servo / servo

Join GitHub today

Convert imports in generate_imports into one per line, so that it can… #13070

Convert imports in generate_imports into one per line, so that it can… #13070

Conversation

tp6vup54 commented Aug 27, 2016 • edited by larsbergstrom

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.

This comment has been minimized.

This comment has been minimized.

KiChjang Aug 27, 2016 • edited

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.

KiChjang commented Aug 27, 2016

jdm commented Aug 27, 2016

bors-servo commented Aug 27, 2016

bors-servo commented Aug 27, 2016

bors-servo commented Aug 27, 2016

highfive commented Aug 27, 2016

KiChjang commented Aug 27, 2016

bors-servo commented Aug 27, 2016

bors-servo commented Aug 27, 2016

tp6vup54 commented Aug 27, 2016 •

edited by larsbergstrom

KiChjang Aug 27, 2016 •

edited