Skip to content

Conversation

@AurelioDeRosa
Copy link
Member

Closes: gh-730

entries/data.xml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Follow the jQuery style guide: {"my-name": "aValue"} => { "my-name": "aValue" }

Copy link
Member

Choose a reason for hiding this comment

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

ditto in the other sentence.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it'd be useful to mention that not everything is camelCased, only /-([a-z])/ -> $1, e.g. a-b-3-c is changed to aB-3C.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mzgol Do you have a suggestion on the wording here? I think many users won't really like or understand the use of a regex in the description.

Copy link
Member

Choose a reason for hiding this comment

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

Every two-character sequence of "-" (U+002D) followed by a lowercase ASCII letter is replaced by the uppercase version of the letter. It's the algorithm defined at http://www.w3.org/TR/html5/dom.html#dom-dataset.

Copy link
Member

Choose a reason for hiding this comment

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

@AurelioDeRosa Yep, if we want to be specific here (and IMO we should, even if we just add a note explaining what camelCasing means here), what @gibson042 said is the most accurate.

PR updated based on feedback.
@arthurvr
Copy link
Member

Why does CI fail?

@arthurvr
Copy link
Member

oh nevermind. We don't have a travis config in the v3 branch yet.

@AurelioDeRosa
Copy link
Member Author

@arthurvr I was wondering what was the reason as well.

@arthurvr
Copy link
Member

Simply because our Travis CI config isn't in the V3 branch yet. I'll rebase that branch this evening.

PR updated based on the last feedback
@AurelioDeRosa
Copy link
Member Author

Anyone keen to review so that we can merge this PR? // cc @arthurvr

entries/data.xml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this note be changed in the same way as the upper one?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh...yes. I'll do it now.

Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to avoid duplication, define it in one place & just use in these two? We do have includes but preferably the pattern would be defined here since it's not needed on other pages. Is it possible?

Copy link
Member Author

Choose a reason for hiding this comment

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

Apart from the global notes I'm not aware of any other method, sorry.

PR updated based on the feedback
Fixed typo
AurelioDeRosa added a commit that referenced this pull request Jul 20, 2015
Document changes that align the method to the Dataset API's behavior

Fixes gh-730
Closes gh-758
@AurelioDeRosa
Copy link
Member Author

Closed with commit 270b518.

@AurelioDeRosa AurelioDeRosa deleted the AurelioDeRosa-data branch July 20, 2015 00:31
AurelioDeRosa added a commit that referenced this pull request May 23, 2016
Document changes that align the method to the Dataset API's behavior

Fixes gh-730
Closes gh-758
timmywil pushed a commit that referenced this pull request Jun 9, 2016
Document changes that align the method to the Dataset API's behavior

Fixes gh-730
Closes gh-758
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