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

Refactor style lengths per #13584 #14077

Merged
merged 1 commit into from Nov 6, 2016
Merged

Refactor style lengths per #13584 #14077

merged 1 commit into from Nov 6, 2016

Conversation

@kivikakk
Copy link
Contributor

kivikakk commented Nov 5, 2016 •
edited by larsbergstrom

Refactor components/style/values/{computed,specified}/mod.rs into …/length.rs per #13584.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #13584
  • These changes do not require tests because the existing code should be covered by tests

This change is Reviewable
@kivikakk
Copy link
Contributor Author

kivikakk commented Nov 5, 2016

I had to make a few more things public to be accessible from the parent module (or vice-versa); not sure how I feel about that.
@emilio
Copy link
Member

emilio commented Nov 5, 2016

Hi, thanks for doing this!

This looks good to me, and even though I agree making some stuff public so the inner modules can access it is not great, it's not too bad (and it'll become better with the pub restricted thing eventually: rust-lang/rfcs#1422).

I don't think we want to delay landing this PR given it's sort of prone to bitrot, thanks a bunch for doing this! :)

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

bors-servo commented Nov 5, 2016

📌 Commit e7a94a0 has been approved by emilio
@bors-servo
Copy link
Contributor

bors-servo commented Nov 5, 2016

⌛ Testing commit e7a94a0 with merge 5be0a31...
bors-servo added a commit that referenced this pull request Nov 5, 2016
@bors-servo
Auto merge of #14077 - kivikakk:refactor-style-lengths, r=emilio
Loading status checks…
5be0a31 
Refactor style lengths per #13584

<!-- Please describe your changes on the following line: -->
Refactor `components/style/values/{computed,specified}/mod.rs` into `…/length.rs` per #13584.

---
<!-- 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 #13584

<!-- Either: -->
- [x] These changes do not require tests because the existing code should be covered by tests

<!-- 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/14077)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 5, 2016

💔 Test failed - linux-dev
@emilio
Copy link
Member

emilio commented Nov 5, 2016

Ouch, this breaks the geckolib build, could you fix that? (should be easy, they're only warnings)

To test it you can do ./mach build-geckolib.

Thanks!
@bors-servo
Copy link
Contributor

bors-servo commented Nov 5, 2016

☔ The latest upstream changes (presumably #13202) made this pull request unmergeable. Please resolve the merge conflicts.
@kivikakk
Copy link
Contributor Author

kivikakk commented Nov 6, 2016

@emilio; will do; thanks for the heads up!
Yuki Izumi
Refactor style lengths per #13584
Unverified
No user is associated with the committer email.
Learn about signing commits
Loading status checks…
f33f560
@kivikakk kivikakk force-pushed the kivikakk:refactor-style-lengths branch from e7a94a0 to f33f560 Nov 6, 2016
@kivikakk
Copy link
Contributor Author

kivikakk commented Nov 6, 2016 •
edited

Rebased with care, and geckolib build fixed. 👍
@kivikakk
Copy link
Contributor Author

kivikakk commented Nov 6, 2016 •
edited

AppVeyor gnu/msys build failed with:

CalledProcessError: Command '['git', 'rev-parse', '--short', 'HEAD']' returned non-zero exit status 127

.. which is strange.

edit: ah, I see this might perhaps be the norm for this particular build.
@emilio
Copy link
Member

emilio commented Nov 6, 2016

Yeah, don't bother about appveyor, we don't gate on it and it's been broken for a few days now. r=me, thanks again! :)

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

bors-servo commented Nov 6, 2016

📌 Commit f33f560 has been approved by emilio
@bors-servo
Copy link
Contributor

bors-servo commented Nov 6, 2016

⌛ Testing commit f33f560 with merge 8c35f93...
bors-servo added a commit that referenced this pull request Nov 6, 2016
@bors-servo
Auto merge of #14077 - kivikakk:refactor-style-lengths, r=emilio
Loading status checks…
8c35f93 
Refactor style lengths per #13584

<!-- Please describe your changes on the following line: -->
Refactor `components/style/values/{computed,specified}/mod.rs` into `…/length.rs` per #13584.

---
<!-- 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 #13584

<!-- Either: -->
- [x] These changes do not require tests because the existing code should be covered by tests

<!-- 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/14077)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 6, 2016

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev
@bors-servo bors-servo merged commit f33f560 into servo:master Nov 6, 2016
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
This was referenced Nov 6, 2016
Merged
Merged
@kivikakk kivikakk deleted the kivikakk:refactor-style-lengths branch Nov 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@emilio emilio

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Split length values into their own file
4 participants
@kivikakk @emilio @bors-servo @highfive
You can’t perform that action at this time.