Skip to content
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

Use more idiomatic or_else construction #15815

Merged
merged 1 commit into from Mar 3, 2017

Conversation

Projects
None yet
5 participants
@crypto-universe
Copy link
Contributor

commented Mar 3, 2017

or_else in more ideomatic rust construction than
match, where Some(x) => Some(x)

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #15812 (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

commented Mar 3, 2017

Heads up! This PR modifies the following files:

  • @fitzgen: components/script/layout_wrapper.rs
  • @KiChjang: components/script/layout_wrapper.rs
@highfive

This comment has been minimized.

Copy link

commented Mar 3, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify script code, but no tests are modified. Please consider adding a test!
@KiChjang
Copy link
Member

left a comment

Great work! The code styling looks correct, but if you just address one minor nit, we can land this!

Some(next) => Some(next),
None => self.parent_node.get_after_pseudo(),
}
(unsafe { node.dangerous_next_sibling() }).or_else(|| self.parent_node.get_after_pseudo())

This comment has been minimized.

Copy link
@KiChjang

KiChjang Mar 3, 2017

Member

nit: The parenthesis around unsafe should be unnecessary.

@KiChjang

This comment has been minimized.

Copy link
Member

commented Mar 3, 2017

Also, there's a typo in the commit message, idiomatic instead of ideomatic.

@crypto-universe crypto-universe force-pushed the crypto-universe:master branch from 7632b30 to 96dbd31 Mar 3, 2017

@crypto-universe

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2017

@KiChjang fixed. Check PR now.

@crypto-universe crypto-universe changed the title Use more ideomatic or_else construction Use more idiomatic or_else construction Mar 3, 2017

Use more idiomatic construction
or_else in more idiomatic rust construction than
match, where Some(x) => Some(x)

@crypto-universe crypto-universe force-pushed the crypto-universe:master branch from 96dbd31 to afd3030 Mar 3, 2017

@KiChjang

This comment has been minimized.

Copy link
Member

commented Mar 3, 2017

@bors-servo r+

Thanks!

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Mar 3, 2017

📌 Commit afd3030 has been approved by KiChjang

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Mar 3, 2017

⌛️ Testing commit afd3030 with merge b4d7ed7...

bors-servo added a commit that referenced this pull request Mar 3, 2017

Auto merge of #15815 - crypto-universe:master, r=KiChjang
Use more idiomatic or_else construction

<!-- Please describe your changes on the following line: -->
or_else in more ideomatic rust construction than
match, where Some(x) => Some(x)
---
<!-- 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 #15812 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/15815)
<!-- Reviewable:end -->
@crypto-universe

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2017

Thank you!

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Mar 3, 2017

☀️ 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
Approved by: KiChjang
Pushing b4d7ed7 to master...

@bors-servo bors-servo merged commit afd3030 into servo:master Mar 3, 2017

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.