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

(docs) Fiber.abort can be passed an object #1015

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

joshgoebel
Copy link

No description provided.

@ChayimFriedman2
Copy link
Contributor

There are some redundant whitespace changes in this PR.

@joshgoebel
Copy link
Author

joshgoebel commented May 12, 2021

We need an .editor_config and some agreement on how excess whitespace is handled (in general) so we can fix this once and for all... trailing whitespace issues is a unnecessary frustration when just trying to make a PR for a small fix.

Other projects I work on assume auto-removal of trailing spaces, and I think that's a good default usually.

@cxw42
Copy link
Contributor

cxw42 commented May 13, 2021

Related to #982

editorconfig

#846 ! I agree with removing all the trailing spaces.

@@ -8,12 +8,16 @@ A lightweight coroutine. [Here][fibers] is a gentle introduction.

### Fiber.**abort**(message)

Raises a runtime error with the provided message:
Raises a runtime error with the provided message. `message` may also be an object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

message, in fact, can be any Wren value, as far as I know. See src/vm/wren_value.h, struct ObjFiber --- it has a Value error. Would you be willing to update the suggested language to note that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can click the +/- and propose specific changes... I'm fine with whatever here as long as it adds clarity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you, please? My Wren time is currently taken up by the switch-statement PR. I appreciate it!

@joshgoebel
Copy link
Author

I took another pass at this.

@ChayimFriedman2
Copy link
Contributor

Better now.

Copy link
Contributor

@cxw42 cxw42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Co-authored-by: Rob Loach <robloach@gmail.com>
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

4 participants