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

Clarifies distinctions between getter and setter methods #656

Closed
wants to merge 1 commit into from

Conversation

lady3bean
Copy link
Contributor

Fixes gh-655

@gnarf
Copy link
Member

gnarf commented Jun 8, 2015

LGTM - cc @jquery/content for a second reviewer please

$( "h1" ).html( "hello world" );
```

```
// The .html() method used as a getter:
$( "h1" ).html();
// The .html() method returns the html from the first h1 element:
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps better "the html of the first"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, that sounds better.

@agcolom
Copy link
Member

agcolom commented Jun 8, 2015

I definitely like the proposed text!
As regards to the comments on the same line, I thought that was ok for code output but I can't find it in the style guide so we'll wait for @arthurvr 's feedback on that. If the outcome is that we have agreed this is ok, then we must remember to update the http://contribute.jquery.org/style-guide/js/

@gnarf
Copy link
Member

gnarf commented Jun 8, 2015

I think it's not actually a "js" exception, but rather a "documentation / content" exception to that rule... We should probably list it on a "Documentation/Content Style Guide" along with the prose rules, etc. Link to and list an exception to the JS Style Guide.

@arthurvr
Copy link
Member

arthurvr commented Jun 8, 2015

We do have an open contribute.jquery.org PR that adds a prose/documentation style guide, this will be something to include there. I'm petty sure we had this whole discussion earlier, let me search for it tomorrow.

@agcolom
Copy link
Member

agcolom commented Jun 8, 2015

I believe this is where we had this discussion: jquery/contribute.jquery.org#75 (comment)

We agreed to use // > on its own line after the code, see details at globalizejs/globalize#313

@lady3bean
Copy link
Contributor Author

Thanks for the input. I've updated it to reflect this style.

@gnarf
Copy link
Member

gnarf commented Jun 8, 2015

I'm totally 👍 here - waiting for someone else to confirm before landing.

@AurelioDeRosa
Copy link
Member

LGTM 👍

@gnarf gnarf closed this in 5eaa312 Jun 8, 2015
$( "h1" ).html();
// > "hello world"

Copy link
Member

Choose a reason for hiding this comment

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

Why is this empty line here?

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

Successfully merging this pull request may close these issues.

6 participants