Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

added missing function okToGreet #5878

Closed
wants to merge 1 commit into from

Conversation

letsmakesense
Copy link
Contributor

the function okToGreet wasn't defined, so this example wouldn't work properly.

the function okToGreet wasn't defined, so this example wouldn't work properly.
@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let us know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@caitp
Copy link
Contributor

caitp commented Jan 18, 2014

Hey, I'm not totally sure this adds a whole lot... I'm not opposed to it necessarily, but I think the okToGreet() function should be defined underneath the asyncGreet() function, just so that the focus is on the original example.

Cool?

@caitp caitp closed this in 190c4f9 Jan 18, 2014
@caitp caitp reopened this Jan 18, 2014
@caitp
Copy link
Contributor

caitp commented Jan 18, 2014

Sorry about that, I accidentally referenced the wrong issue... my mistake :[

@letsmakesense
Copy link
Contributor Author

no problem . the issue isn't that important. just to avoid that more
people stumble upon it.

On Sat, 18 Jan 2014 16:12:13 -0600, Caitlin Potter
notifications@github.com wrote:

Sorry about that, I accidentally referenced the wrong issue... my
mistake :[


Reply to this email directly or view it on GitHub:
#5878 (comment)

@caitp
Copy link
Contributor

caitp commented Jan 18, 2014

So, I've decided that I don't think defining this function in the example really adds anything useful to the example, but I think a note could be made of it (that it's expected to be in the lexical scope), does that work for you?

@letsmakesense
Copy link
Contributor Author

yeah, that's fine, too

On Sat, 18 Jan 2014 16:32:37 -0600, Caitlin Potter
notifications@github.com wrote:

So, I've decided that I don't think defining this function in the
example really adds anything useful to the example, but I think a note
could be made of it (that it's expected to be in the lexical scope),
does that work for you?


Reply to this email directly or view it on GitHub:
#5878 (comment)

@letsmakesense
Copy link
Contributor Author

you could also omit the function completely:

if (okToGreet(name))
< if (name == 'Robin Hood')

to avoid unnecessary boilerplate notes.

On Sat, 18 Jan 2014 16:32:37 -0600, Caitlin Potter
notifications@github.com wrote:

So, I've decided that I don't think defining this function in the
example really adds anything useful to the example, but I think a note
could be made of it (that it's expected to be in the lexical scope),
does that work for you?


Reply to this email directly or view it on GitHub:
#5878 (comment)

@caitp caitp closed this in 90e60d2 Jan 18, 2014
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
…xample

the function okToGreet wasn't defined, so this example wouldn't work properly.

I've decided that instead of adding unrelated code to the example, it should just be noted that the
function is expected to be defined in the lexical scope.

Closes angular#5878
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
…xample

the function okToGreet wasn't defined, so this example wouldn't work properly.

I've decided that instead of adding unrelated code to the example, it should just be noted that the
function is expected to be defined in the lexical scope.

Closes angular#5878
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants