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

Refactor geckolib atoms regen script #13476

Merged
merged 2 commits into from Sep 30, 2016
Merged

Refactor geckolib atoms regen script #13476

merged 2 commits into from Sep 30, 2016

Conversation

@wafflespeanut
Copy link
Member

wafflespeanut commented Sep 28, 2016


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because it's a refactor

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

highfive commented Sep 28, 2016

Heads up! This PR modifies the following files:

  • @bholley: components/style/binding_tools/regen_atoms.py, components/style/gecko_string_cache/atom_macro.rs
@highfive

This comment has been minimized.

Copy link

highfive commented Sep 28, 2016

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify style code, but no tests are modified. Please consider adding a test!
@wafflespeanut

This comment has been minimized.

Copy link
Member Author

wafflespeanut commented Sep 28, 2016

r? @emilio

@highfive highfive assigned emilio and unassigned mbrubeck Sep 28, 2016
@emilio

This comment has been minimized.

Copy link
Member

emilio commented Sep 28, 2016

This looks good to me, though can we use str.format() instead of the % operator, so it works in both python2 and python3? I know we're not probably making the switch anytime soon, but...

@wafflespeanut wafflespeanut force-pushed the wafflespeanut:atoms branch from a8c1720 to 6f499e4 Sep 28, 2016
@wafflespeanut

This comment has been minimized.

Copy link
Member Author

wafflespeanut commented Sep 28, 2016

I was lazy about having double curly braces everywhere. I've made the changes now 😄

@emilio

This comment has been minimized.

Copy link
Member

emilio commented Sep 28, 2016

Neat, thanks!

@bors-servo: r+

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Sep 28, 2016

📌 Commit 6f499e4 has been approved by emilio

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Sep 30, 2016

☔️ The latest upstream changes (presumably #13477) made this pull request unmergeable. Please resolve the merge conflicts.

@Manishearth Manishearth force-pushed the wafflespeanut:atoms branch from 6f499e4 to 77442e8 Sep 30, 2016
@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Sep 30, 2016

Fixed, and included missing stuff from the regen

@bors-servo r+

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Sep 30, 2016

📌 Commit 77442e8 has been approved by Manishearth

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Sep 30, 2016

💡 This pull request was already approved, no need to approve it again.

  • There's another pull request that is currently being tested, blocking this pull request: #13473
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Sep 30, 2016

📌 Commit 77442e8 has been approved by Manishearth

@Manishearth Manishearth force-pushed the wafflespeanut:atoms branch from 77442e8 to b6fccbf Sep 30, 2016
@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Sep 30, 2016

@bors-servo r=Manishearth,emilio

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Sep 30, 2016

📌 Commit b6fccbf has been approved by Manishearth,emilio

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Sep 30, 2016

⌛️ Testing commit b6fccbf with merge 4101260...

bors-servo added a commit that referenced this pull request Sep 30, 2016
Refactor geckolib atoms regen script

<!-- Please describe your changes on the following line: -->

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

<!-- Either: -->
- [x] These changes do not require tests because it's a refactor

<!-- 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/13476)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Sep 30, 2016

@bors-servo bors-servo merged commit b6fccbf into servo:master Sep 30, 2016
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.