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

Random fixups #5992

Merged
merged 4 commits into from May 12, 2015
Merged

Random fixups #5992

merged 4 commits into from May 12, 2015
+44 −27

Conversation

@zmike
Copy link
Contributor

zmike commented May 8, 2015

Attempt to not panic as much if the resources/ dir is not where it's expected to be.

Review on Reviewable
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented May 8, 2015

Critic review: https://critic.hoppipolla.co.uk/r/4945

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.
@Ms2ger
Copy link
Contributor

Ms2ger commented May 8, 2015

I'm not sure about this. The UA style sheets are an essential part of the browser; you can't just run without them.
@zmike
Copy link
Contributor Author

zmike commented May 8, 2015

Then I guess it should at least print an error instead of just panicking with no info?
@pcwalton
Copy link
Contributor

pcwalton commented May 8, 2015

I agree with both of you; print an error, say what's wrong, and exit gracefully instead of proceeding silently.
@zmike zmike force-pushed the zmike:random-fixups branch from 456e33e to 0f423fa May 8, 2015
@zmike
Copy link
Contributor Author

zmike commented May 8, 2015

Seems reasonable. Done.
@zmike zmike force-pushed the zmike:random-fixups branch 2 times, most recently from d66426a to 05c152e May 9, 2015
@bors-servo
Copy link
Contributor

bors-servo commented May 12, 2015

☔ The latest upstream changes (presumably #6010) made this pull request unmergeable. Please resolve the merge conflicts.
Mike Blumenkrantz added 4 commits May 8, 2015
Mike Blumenkrantz
fix Stylist::new() to not panic when resources cannot be loaded
1cf5dd2
Mike Blumenkrantz
fix Stylist::add_quirks_mode_stylesheet() to not panic when resources…
0824793 
… cannot be loaded
Mike Blumenkrantz
when finding resource_dir, try exe launch directory
ac6aa9b 
external apps using servo may just have the resources/ in the same place
Mike Blumenkrantz
unset resources path on linux in embedding crate
Loading status checks…
a838318 
this blocked execution on linux due to bad paths
@zmike zmike force-pushed the zmike:random-fixups branch from 05c152e to a838318 May 12, 2015
@larsbergstrom
Copy link
Contributor

larsbergstrom commented May 12, 2015

Reviewed files:

  • components/style/selector_matching.rs @ r5
  • components/util/resource_files.rs @ r3
  • ports/cef/core.rs @ r5

Comments from the review on Reviewable.io
@larsbergstrom
Copy link
Contributor

larsbergstrom commented May 12, 2015

@bors-servo: r+
@bors-servo
Copy link
Contributor

bors-servo commented May 12, 2015

📌 Commit a838318 has been approved by larsbergstrom
@bors-servo
Copy link
Contributor

bors-servo commented May 12, 2015

⌛ Testing commit a838318 with merge 76225bd...
bors-servo pushed a commit that referenced this pull request May 12, 2015
bors-servo
Auto merge of #5992 - zmike:random-fixups, r=larsbergstrom
Loading status checks…
76225bd 
Attempt to not panic as much if the resources/ dir is not where it's expected to be.

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

bors-servo commented May 12, 2015

☀️ Test successful - android, gonk, linux1, linux2, mac1, mac2
@bors-servo bors-servo merged commit a838318 into servo:master May 12, 2015
2 checks passed
2 checks passed
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
Reviewers
No reviews
Assignees
No one assigned
Labels
S-awaiting-review
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
@zmike @hoppipolla-critic-bot @Ms2ger @pcwalton @bors-servo @larsbergstrom @highfive
You can’t perform that action at this time.