Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Update scope.md #452

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
7 participants

Added a case where the scopes might cause an error based on a previous case.

Owner

scottgonzalez commented Apr 10, 2014

The last comment in the example is wrong.

I'm not sure this additional example is needed, but I can see some value in it. I'd like to hear thoughts from some others.

You are right, I forgot to change that inline comment so it's contradicting the block text from before. However, could it be considered a small typo? Waiting to see others response anyway (:

Owner

arschmitz commented May 12, 2014

While I agree with @scottgonzalez that I don't think this is needed. I don't see the harm in adding it if anyone thinks it will clarify for readers.

Member

tjvantoll commented May 12, 2014

I actually like the additional example, but the sentence leading into it doesn't make it clear what has changed. I found myself manually comparing code to see what had changed. I would consider something like:

Here, the var statement has been removed from the foo = "world" assignment. Therefore, instead of declaring a new local variable, the global foo variable is updated. This code outputs "hello" twice:

Owner

arschmitz commented May 12, 2014

+1 on the updated intro sentence

Member

kswedberg commented May 12, 2014

I like the updated intro sentence, too. However, the verb tense of the first sentence makes it hard to know which code block you're referring to. "Here, the var statement has been removed..." sounds like it's referring to the previous example.

@tjvantoll : Maybe the first sentence would be more clear like this?

In the following example, the var statement is removed from the foo = "world" assignment.

Member

tjvantoll commented May 12, 2014

@kswedberg +1 to the updated sense. It's more clear. So combined...

In the following example, the var statement is removed from the foo = "world" assignment. Therefore, instead of declaring a new local variable, the global foo variable is updated. This code outputs "hello" twice:

@franciscop Does that look alright to you? If so, could you add a commit that updates the wording and fixes the typo in the code block?

@tjvantoll of course, I'm not native so I apologize for the trouble. Please feel completely free to edit the text, I just thought that the additional case would be interesting and educational. I will add it now.

franciscop added some commits May 13, 2014

@franciscop franciscop Update scope.md
Updated text as suggested here: #452
d93cc90
@franciscop franciscop Update scope.md
Added markdown to the last change (didn't get copied when I pasted it here).
f7060d6
Member

tjvantoll commented May 14, 2014

👍

Owner

scottgonzalez commented May 14, 2014

@franciscop Please sign our CLA so we can land this. Thanks.

I already signed it, but now I just did with my github public email. Thank you so much.

Member

arthurvr commented Dec 5, 2014

Please sign our CLA so we can land this.

Can this now be merged? Open since the beginning of this year...

Owner

dmethvin commented Dec 12, 2014

Looks like this was clarified after this PR was filed so I will close it.

@dmethvin dmethvin closed this Dec 12, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment