Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGlobalScope::current should return Option #17238
Comments
|
I would like to work on this, considering I've been working on the PR where this was originated. |
|
Sure! I expect in many cases we can just attach |
|
Sounds good, I'll be on it in the weekend. If you think you'd like to do this earlier, I'd have no problems with that :) |
|
Nope, no hurry! |
|
@brainlessdeveloper Have you made progress on this? |
|
Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the If you intend to work on this issue, then add |
|
I've been quite busy recently, I wouldn't mind if you unassign me. I have to complete #16508 too. |
|
@highfive: assign me I have a PR waiting for review, but would like to take a look at this in the meantime.. if that's alright? |
|
Hey @moonlightdrive! Thanks for your interest in working on this issue. It's now assigned to you! |
|
Yes, please do. I'm going to be dealing with that PR soon; sorry for the delay. |
|
@jdm no worries! How should I verify that my changes work? Is there a particular test I should run? |
|
If you make the API return Option and use expect() in every existing case, there will be no functional change. |
…sajeffrey return Option from GlobalScope::current <!-- Please describe your changes on the following line: --> handles the case where `GlobalScope::current` calls `CurrentGlobalOrNull` and the result is null --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #17238 <!-- Either: --> - [ ] There are tests for these changes OR - [X] These changes do not require tests because: no functionality change & ./mach build -d has no errors <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17472) <!-- Reviewable:end -->
It invokes CurrentGlobalOrNull but does not actually handle the null case and asserts in global_scope_from_global.