Skip to content

Add "before" and "after" atoms. - #141

Closed
emilio wants to merge 1 commit into
servo:masterfrom
emilio:pseudo-elements
Closed

Add "before" and "after" atoms.#141
emilio wants to merge 1 commit into
servo:masterfrom
emilio:pseudo-elements

Conversation

@emilio

@emilio emilio commented Feb 8, 2016

Copy link
Copy Markdown
Member

Needed for servo/servo#9567

Review on Reviewable

@emilio

emilio commented Feb 8, 2016

Copy link
Copy Markdown
Member Author

r? @SimonSapin

@KiChjang

KiChjang commented Feb 8, 2016

Copy link
Copy Markdown
Contributor

Version bump!

@asajeffrey

Copy link
Copy Markdown

Looks good. Can you squash the two commits into one?


Reviewed 1 of 1 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@asajeffrey asajeffrey self-assigned this Feb 8, 2016
@KiChjang

KiChjang commented Feb 8, 2016

Copy link
Copy Markdown
Contributor

I disagree with squashing, because we wouldn't be able to tell from the commit message which commit the version bump happened.

@asajeffrey

Copy link
Copy Markdown

Fine by me.

@bors-servo: r+

@bors-servo

Copy link
Copy Markdown
Contributor

📌 Commit e6377bb has been approved by asajeffrey

@bors-servo

Copy link
Copy Markdown
Contributor

⌛ Testing commit e6377bb with merge 2bec438...

bors-servo pushed a commit that referenced this pull request Feb 8, 2016
Add "before" and "after" atoms.

Needed for servo/servo#9567

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/string-cache/141)
<!-- Reviewable:end -->
@emilio

emilio commented Feb 8, 2016

Copy link
Copy Markdown
Member Author

@bors-servo: r- force

@emilio

emilio commented Feb 8, 2016

Copy link
Copy Markdown
Member Author

I'd wait to land this until the approach taken in servo/servo#9567 is decided. There might be a chance this is not needed.

Re: squashing, I don't mind doing it. I think it's fine since it's a one-commit release.

@emilio emilio closed this Feb 8, 2016
@emilio emilio reopened this Feb 8, 2016
@KiChjang

KiChjang commented Feb 8, 2016

Copy link
Copy Markdown
Contributor

git commit --amend followed by a git push -f should fix the status on homu.

@asajeffrey

Copy link
Copy Markdown

OK, though the cost for adding new static atoms is pretty small, and we're quite likely to want before and after at some point.

@emilio

emilio commented Feb 8, 2016

Copy link
Copy Markdown
Member Author

@asajeffrey: Merge it if you want then, I personally won't add unneeded atoms, but I agree these are likely to be desirable at some point :)

@asajeffrey

Copy link
Copy Markdown

We can leave it open for the moment, and wait for the dust to settle on servo/servo#9567.

@bors-servo

Copy link
Copy Markdown
Contributor

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

@emilio

emilio commented Feb 24, 2016

Copy link
Copy Markdown
Member Author

Closing this since servo/servo#9567 implementation didn't finally need this.

@emilio emilio closed this Feb 24, 2016
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.

4 participants