Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Webbluetooth requestDevice refactor and update #13184

Merged
merged 3 commits into from Sep 15, 2016
Merged

Webbluetooth requestDevice refactor and update #13184

merged 3 commits into from Sep 15, 2016
+297 −91

Conversation

@zakorgy
Copy link
Contributor

zakorgy commented Sep 6, 2016 •
edited

Refactor requestDevice function according to the specification changes.

  1. Moved the request_bluetooth_devices algorithm out from the requestDevice function.
  2. Two new members in BluetoothRequestDeviceFilter and one new member in RequestDeviceOptions.
  3. Also added annotations to the related functions.
    Related links:
    https://webbluetoothcg.github.io/web-bluetooth/#device-discovery,
    https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetooth-requestdevice, https://webbluetoothcg.github.io/web-bluetooth/#request-bluetooth-devices,
    https://webbluetoothcg.github.io/web-bluetooth/#matches-a-filter
  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #12614
  • These changes do not require tests because , there is no WebBluetooth Test API implementation yet.

This change is Reviewable
@highfive
Copy link

highfive commented Sep 6, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/net_traits/bluetooth_scanfilter.rs, components/net_traits/bluetooth_scanfilter.rs, components/script/dom/bluetooth.rs, components/script/dom/webidls/Bluetooth.webidl, components/net/bluetooth_thread.rs
@highfive
Copy link

highfive commented Sep 6, 2016

warning Warning warning

  • These commits modify net and script code, but no tests are modified. Please consider adding a test!
@nox
Copy link
Member

nox commented Sep 6, 2016

Could it somehow be split in multiple independent commits?
@zakorgy
Copy link
Contributor Author

zakorgy commented Sep 6, 2016

@nox how do you prefer the splitting? First commit refactor and update, second with the annotations, or should I split it by sub parts with the corresponding annotations e.g. first commit new dict members , second commit requestDevice modification etc. ?
@nox
Copy link
Member

nox commented Sep 6, 2016

Unrelated changes should go in separate commits, and each commit should build correctly. So probably just two or three, one with the clean up, one with the annotations, and finally the fix.
@zakorgy zakorgy force-pushed the szeged:requestdevice-refactor branch from 1c631d7 to 69e27bc Sep 7, 2016
@zakorgy
Copy link
Contributor Author

zakorgy commented Sep 7, 2016

r? @nox @jdm
@bors-servo
Copy link
Contributor

bors-servo commented Sep 9, 2016

☔ The latest upstream changes (presumably #13205) made this pull request unmergeable. Please resolve the merge conflicts.
@zakorgy zakorgy force-pushed the szeged:requestdevice-refactor branch from 69e27bc to 643500c Sep 9, 2016
zakorgy added 3 commits Sep 7, 2016
@zakorgy
requestDevice refactor
057b93f
@zakorgy
requestDevice update
359548f
@zakorgy
Annotations for requestDevice, request_bluetooth_devices and matches_…
Loading status checks…
f4fda68 
…filter functions
@zakorgy zakorgy force-pushed the szeged:requestdevice-refactor branch from 643500c to f4fda68 Sep 14, 2016
@zakorgy
Copy link
Contributor Author

zakorgy commented Sep 15, 2016

r? @jdm
@highfive highfive assigned jdm and unassigned nox Sep 15, 2016
@jdm
Copy link
Member

jdm commented Sep 15, 2016

@bors-servo: r+
Thanks for adding the annotations!
@bors-servo
Copy link
Contributor

bors-servo commented Sep 15, 2016

📌 Commit f4fda68 has been approved by jdm
@bors-servo
Copy link
Contributor

bors-servo commented Sep 15, 2016

⌛ Testing commit f4fda68 with merge d92d699...
bors-servo added a commit that referenced this pull request Sep 15, 2016
@bors-servo
Auto merge of #13184 - szeged:requestdevice-refactor, r=jdm
Loading status checks…
d92d699 
Webbluetooth requestDevice refactor and update

<!-- Please describe your changes on the following line: -->
Refactor requestDevice function according to the specification changes.
1. Moved the `request_bluetooth_devices` algorithm out from the `requestDevice` function.
2. Two new members in `BluetoothRequestDeviceFilter` and one new member in `RequestDeviceOptions`.
3. Also added annotations to the related functions.
Related links:
https://webbluetoothcg.github.io/web-bluetooth/#device-discovery,
https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetooth-requestdevice, https://webbluetoothcg.github.io/web-bluetooth/#request-bluetooth-devices,
https://webbluetoothcg.github.io/web-bluetooth/#matches-a-filter

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #12614

<!-- Either: -->
- [x] These changes do not require tests because , there is no WebBluetooth Test API implementation yet.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13184)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 15, 2016

💔 Test failed - linux-rel
@highfive
Copy link

highfive commented Sep 15, 2016 

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform-table-007.htm
  └   → /css-transforms-1_dev/html/transform-table-007.htm a5c014b20ef1363bea6f24eda28c7efb7c45698a
/css-transforms-1_dev/html/reference/transform-blank-ref.htm fa6407b1acbbfea27e27061e7d1bdeca98e4a728
Testing a5c014b20ef1363bea6f24eda28c7efb7c45698a == fa6407b1acbbfea27e27061e7d1bdeca98e4a728
@jdm
Copy link
Member

jdm commented Sep 15, 2016

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Sep 15, 2016

⌛ Testing commit f4fda68 with merge 8334020...
bors-servo added a commit that referenced this pull request Sep 15, 2016
@bors-servo
Auto merge of #13184 - szeged:requestdevice-refactor, r=jdm
Loading status checks…
8334020 
Webbluetooth requestDevice refactor and update

<!-- Please describe your changes on the following line: -->
Refactor requestDevice function according to the specification changes.
1. Moved the `request_bluetooth_devices` algorithm out from the `requestDevice` function.
2. Two new members in `BluetoothRequestDeviceFilter` and one new member in `RequestDeviceOptions`.
3. Also added annotations to the related functions.
Related links:
https://webbluetoothcg.github.io/web-bluetooth/#device-discovery,
https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetooth-requestdevice, https://webbluetoothcg.github.io/web-bluetooth/#request-bluetooth-devices,
https://webbluetoothcg.github.io/web-bluetooth/#matches-a-filter

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #12614

<!-- Either: -->
- [x] These changes do not require tests because , there is no WebBluetooth Test API implementation yet.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13184)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 15, 2016

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev
@bors-servo bors-servo merged commit f4fda68 into servo:master Sep 15, 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
@zakorgy zakorgy mentioned this pull request Sep 16, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jdm jdm

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Step annotations for files related to WebBluetooth
5 participants
@zakorgy @highfive @nox @bors-servo @jdm
You can’t perform that action at this time.