-
Notifications
You must be signed in to change notification settings - Fork 260
data: Document behavior changes #758
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
Conversation
entries/data.xml
Outdated
There was a problem hiding this comment.
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" }
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
Why does CI fail? |
|
oh nevermind. We don't have a travis config in the v3 branch yet. |
|
@arthurvr I was wondering what was the reason as well. |
|
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
|
Anyone keen to review so that we can merge this PR? // cc @arthurvr |
entries/data.xml
Outdated
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
|
Closed with commit 270b518. |
Closes: gh-730