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 unwrap(), not needed #15468

Closed
wants to merge 1 commit into from
Closed

Remove unwrap(), not needed #15468

wants to merge 1 commit into from

Conversation

@karan1276
Copy link
Contributor

karan1276 commented Feb 9, 2017

Fixes #15387

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because it's not really needed

This change is Reviewable

@highfive
Copy link

highfive commented Feb 9, 2017

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

@highfive
Copy link

highfive commented Feb 9, 2017

Heads up! This PR modifies the following files:

  • @fitzgen: components/script/script_thread.rs
  • @KiChjang: components/script/script_thread.rs
@highfive
Copy link

highfive commented Feb 9, 2017

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@karan1276
Copy link
Contributor Author

karan1276 commented Feb 9, 2017

aaa, i think i need to rebase.

@@ -1590,7 +1590,7 @@ impl ScriptThread {
let chan = &load.layout_chan;
if chan.send(message::Msg::PrepareToExit(response_chan)).is_ok() {
debug!("shutting down layout for page {}", id);
response_port.recv().unwrap();
response_port.recv();

This comment has been minimized.

@wafflespeanut

wafflespeanut Feb 9, 2017

Member

Let's ignore the result here with an anonymous let binding.

This comment has been minimized.

@karan1276

karan1276 Feb 9, 2017

Author Contributor

Oh okay, ill do that

This comment has been minimized.

@karan1276

karan1276 Feb 9, 2017

Author Contributor

Anonymous let will be let response_port.recv(); thats all right?

This comment has been minimized.

@wafflespeanut

wafflespeanut Feb 9, 2017

Member
let _ = response_port.recv()
@karan1276
Copy link
Contributor Author

karan1276 commented Feb 9, 2017

I can't really understand the conflicting code. If someone can tell me how to go about this it will be great. Thanks :)

@wafflespeanut
Copy link
Member

wafflespeanut commented Feb 9, 2017

git checkout master
git pull <upstream> master
git checkout fix_15387
git rebase master

And, it will write the the conflicts to the file. Then, you can update the file with your changes,
git add <file> and finally, git rebase --continue should fix everything :)

@karan1276
Copy link
Contributor Author

karan1276 commented Feb 9, 2017

@wafflespeanut awesome!

@karan1276
Copy link
Contributor Author

karan1276 commented Feb 9, 2017

something went wrong, i messed up my git history. Ill open a new PR

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.

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