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

Return the result from query functions. #8205

Merged
merged 5 commits into from Oct 26, 2015
Merged

Conversation

Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Oct 26, 2015

This reduces some unnecessarily tight coupling, makes it clearer what these functions do, and may help avoid bugs where we would return from such a function without updating the relevant field.

It is also a precondition for some future experimentation I'm thinking of doing with this querying design.

Review on Reviewable

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 26, 2015
@highfive
Copy link

warning Warning warning

  • These commits modify layout code, but no reftests are modified. Please consider adding a reftest!

@jdm
Copy link
Member

jdm commented Oct 26, 2015

@bors-servo: r+
This is a good change. Thanks!

@bors-servo
Copy link
Contributor

📌 Commit 68830ed has been approved by jdm

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 26, 2015
@bors-servo
Copy link
Contributor

⌛ Testing commit 68830ed with merge 3a254b7...

bors-servo pushed a commit that referenced this pull request Oct 26, 2015
Return the result from query functions.

This reduces some unnecessarily tight coupling, makes it clearer what these functions do, and may help avoid bugs where we would return from such a function without updating the relevant field.

It is also a precondition for some future experimentation I'm thinking of doing with this querying design.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8205)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

@bors-servo bors-servo merged commit 68830ed into servo:master Oct 26, 2015
@Ms2ger Ms2ger deleted the query branch October 26, 2015 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants