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

Remove FIXME in handle_evaluate_js #6728 #6755

Closed
wants to merge 4 commits into from
Closed

Conversation

@sgmenda
Copy link
Contributor

sgmenda commented Jul 25, 2015

I removed the comments

Review on Reviewable

@highfive
Copy link

highfive commented Jul 25, 2015

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

@jdm
Copy link
Member

jdm commented Jul 25, 2015

Thanks for the patch! There are a couple changes that will be needed before we can merge this, though :)
-S-awaiting-review +S-needs-code-changes


Reviewed 1 of 1 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.


components/script/devtools.rs, line 42 [r4] (raw file):
Please leave this FIXME; it hasn't been resolved yet. However, feel free to work on #6756 and change that!


components/script/script_task.rs, line 793 [r4] (raw file):
nit: please revert this change.


components/script/script_task.rs, line 814 [r4] (raw file):
nit: revert this change, please.


Comments from the review on Reviewable.io

@metajack
Copy link
Contributor

metajack commented Jul 29, 2015

@s142857 Are you still working on the necessary changes?

@sgmenda
Copy link
Contributor Author

sgmenda commented Jul 29, 2015

I had actually suspended working on it, I'll continue today
On 29 Jul 2015 22:26, "Jack Moffitt" notifications@github.com wrote:

@s142857 https://github.com/s142857 Are you still working on the
necessary changes?


Reply to this email directly or view it on GitHub
#6755 (comment).

@sgmenda
Copy link
Contributor Author

sgmenda commented Jul 29, 2015

If you want to take it up, I committed my changes so you can continue

Thanks
On 29 Jul 2015 22:28, "Sanketh Menda" mendasanketh@gmail.com wrote:

I had actually suspended working on it, I'll continue today
On 29 Jul 2015 22:26, "Jack Moffitt" notifications@github.com wrote:

@s142857 https://github.com/s142857 Are you still working on the
necessary changes?


Reply to this email directly or view it on GitHub
#6755 (comment).

@sgmenda
Copy link
Contributor Author

sgmenda commented Aug 1, 2015

sorry i cant finish it.

@jdm
Copy link
Member

jdm commented Aug 1, 2015

Ok, thanks for letting us know!

@jdm jdm closed this Aug 1, 2015
This was referenced Aug 2, 2015
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

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