Skip to content

Conversation

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Mar 9, 2017

This reverts PR #256 / commit 6089ea4.

Turns out these atoms are needed in servo_atoms::Atom, not LocalName.


This change is Reviewable

@emilio
Copy link
Member

emilio commented Mar 9, 2017

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit cc2f8db has been approved by emilio

@bors-servo
Copy link
Contributor

⌛ Testing commit cc2f8db with merge 2bd834f...

bors-servo pushed a commit that referenced this pull request Mar 9, 2017
Revert "Add "contents" and "scroll-position" keywords to local names"

This reverts PR #256 / commit 6089ea4.

Turns out these atoms are needed in `servo_atoms::Atom`, not `LocalName`.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/html5ever/257)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - status-travis
Approved by: emilio
Pushing 2bd834f to master...

@bors-servo bors-servo merged commit cc2f8db into master Mar 9, 2017
@SimonSapin
Copy link
Member Author

I’ve realized after submitting this that removing static atoms is technically a breaking change, and v0.2.1 is already on crates.io with them. @nox, what approach would you prefer?

  • Keep the useless static atoms (revert this revert)
  • Yank 0.2.1 (it was published 4 days ago and I highly doubt anyone depends on the new static atoms) and pretend it never happened
  • Move to 0.3.0, which means h5e also gets a breaking release.

@KiChjang
Copy link
Contributor

KiChjang commented Mar 9, 2017

I think yanking would be the correct approach here.

@SimonSapin
Copy link
Member Author

I tend to agree. Yanked. (We can still unyank or/and do 0.3.0.)

@SimonSapin SimonSapin deleted the revert-256 branch March 9, 2017 21:50
@nox
Copy link
Contributor

nox commented Mar 9, 2017

@SimonSapin I think we are supposed to release a 0.2.2 without the change that we yanked, otherwise even crates.io is confused.

@SimonSapin
Copy link
Member Author

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants