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

implementing step 12 of 'prepare a script' algorithm #4865

Merged
merged 1 commit into from Feb 21, 2015

Conversation

psdh
Copy link
Contributor

@psdh psdh commented Feb 7, 2015

@highfive
Copy link

highfive commented Feb 7, 2015

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @metajack (or someone else) soon.

@hoppipolla-critic-bot
Copy link

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

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 Feb 7, 2015

@psdh: looks pretty good; I left some comments on critic.

@jdm jdm added the S-needs-code-changes Changes have not yet been made that were requested by a reviewer. label Feb 7, 2015
@psdh
Copy link
Contributor Author

psdh commented Feb 18, 2015

Sorry for the delay on this one, it just slipped out of my mind
can someone please help me correct https://critic.hoppipolla.co.uk/showcomment?chain=10458

@jdm
Copy link
Member

jdm commented Feb 18, 2015

@psdh The code linked there does not appear to be part of the algorithm described in the original issue, right?

@Ms2ger Ms2ger added S-needs-squash Some (or all) of the commits in the PR should be combined. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Feb 20, 2015
bors-servo pushed a commit that referenced this pull request Feb 20, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Feb 20, 2015

@psdh: another passing test!

/html/semantics/scripting-1/the-script-element/script-for-onload.html
---------------------------------------------------------------------
PASS expected FAIL Script for and onload attributes

@Ms2ger Ms2ger added S-tests-failed The changes caused existing tests to fail. and removed S-needs-squash Some (or all) of the commits in the PR should be combined. labels Feb 20, 2015
bors-servo pushed a commit that referenced this pull request Feb 21, 2015
@bors-servo bors-servo closed this Feb 21, 2015
@bors-servo bors-servo merged commit 44e2e27 into servo:master Feb 21, 2015
@psdh psdh deleted the prepscript branch February 26, 2015 08:08
@SimonSapin SimonSapin removed the S-tests-failed The changes caused existing tests to fail. label Jan 27, 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.

None yet

7 participants