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

Fix #3933 #3938

Merged
merged 1 commit into from Nov 12, 2014
Merged

Fix #3933 #3938

merged 1 commit into from Nov 12, 2014

Conversation

@guillaumebort
Copy link
Contributor

guillaumebort commented Nov 7, 2014

Implement HTMLOptionElement.{label,value}

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Nov 7, 2014

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

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 Nov 7, 2014

I don't think there's a need to add those fields. Note that @mvanderh is also working on this issue.

@guillaumebort
Copy link
Contributor Author

guillaumebort commented Nov 8, 2014

Ok you are right, these fields were not needed.

I had a look at what @mvanderh was doing and I discovered how wpt/metadata/*.ini files were working... so I fixed them as well.

This is actually my first look at the servo codebase, so I'm not sure at all of what I'm doing. I let this open for now.

@jdm
Copy link
Member

jdm commented Nov 11, 2014

Looking good! I've left one comment on the Critic review.

@guillaumebort
Copy link
Contributor Author

guillaumebort commented Nov 12, 2014

@jdm ok, I've removed the useless call to force_layout.

@jdm
Copy link
Member

jdm commented Nov 12, 2014

Great! Please squash and I'll merge :)

@guillaumebort
Copy link
Contributor Author

guillaumebort commented Nov 12, 2014

@jdm Done

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented on 0976651 Nov 12, 2014

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 12, 2014

merging guillaumebort/servo/fix/3933 = 0976651 into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 12, 2014

guillaumebort/servo/fix/3933 = 0976651 merged ok, testing candidate = 156db28

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 12, 2014

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 12, 2014

merging guillaumebort/servo/fix/3933 = 0976651 into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 12, 2014

guillaumebort/servo/fix/3933 = 0976651 merged ok, testing candidate = 668d921

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

bors-servo replied Nov 12, 2014

fast-forwarding master to auto = 668d921

bors-servo pushed a commit that referenced this pull request Nov 12, 2014
Implement HTMLOptionElement.{label,value}
bors-servo pushed a commit that referenced this pull request Nov 12, 2014
Implement HTMLOptionElement.{label,value}
@bors-servo bors-servo closed this Nov 12, 2014
@bors-servo bors-servo merged commit 0976651 into servo:master Nov 12, 2014
1 check passed
1 check passed
default all tests passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.