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

Editorial: turn "exceptions enabled" into a flag #1134

Merged
merged 2 commits into from
May 2, 2016
Merged

Conversation

annevk
Copy link
Member

@annevk annevk commented Apr 27, 2016

No description provided.

@annevk
Copy link
Member Author

annevk commented Apr 27, 2016

I also added "Rethrow any exceptions" to all callers.

@annevk
Copy link
Member Author

annevk commented Apr 28, 2016

This is blocking #1130 since that wants to turn another inexplicit parameter into an explicit one. Would be easier if this landed first and if this requires a different approach I'd rather sort that out first.

@domenic domenic added the clarification Standard could be clearer label Apr 28, 2016
@domenic
Copy link
Member

domenic commented Apr 29, 2016

A bunch of other flags in the document seem to have their <dfn> and <var>/<span> references include the word "flag". A bunch more don't. It's maybe half-half.

I prefer it with "flag" included; you're defining the "exceptions enabled flag", not "exceptions enabled". It would be good to change this here, and maybe in the future try to uniformize things.

LGTM otherwise.

@annevk
Copy link
Member Author

annevk commented Apr 30, 2016

You're fine with linking back to the argument? I wasn't super sure on that part myself.

@domenic
Copy link
Member

domenic commented Apr 30, 2016

I'm not so sure either; it's a bit inconsistent, but it seems better than the cases where we don't. Maybe another thing to do more of in the future...

@annevk
Copy link
Member Author

annevk commented May 1, 2016

It makes it easier to find the various call sites which use it which for such a widely referenced algorithm is quite nice.

@annevk annevk merged commit 879edbf into master May 2, 2016
@annevk annevk deleted the navigate-exceptions branch May 2, 2016 07:13
@annevk
Copy link
Member Author

annevk commented May 2, 2016

Always easy to remove the <dfn> and <span>s if we change our mind later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer
Development

Successfully merging this pull request may close these issues.

2 participants