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

Longhand labels key in getting started #9465

Merged
merged 2 commits into from Jul 27, 2021
Merged

Longhand labels key in getting started #9465

merged 2 commits into from Jul 27, 2021

Conversation

Yash-Singh1
Copy link
Contributor

No description provided.

etimberg
etimberg previously approved these changes Jul 24, 2021
@LeeLenaleee
Copy link
Collaborator

LeeLenaleee commented Jul 24, 2021

Even though it is valid js I dont think this should be shown like this, I have seen this issue come by at least once here in git and once in stack that people tried to pass data in the shorthand way and didnt understand why it didnt work so to keep it more clear I think its better to just show the full way.

EDIT:
If this is preffered over the long way I think it might be better to change all the places you can shorten it in the documentation for sake of consistency.

@kurkle
Copy link
Member

kurkle commented Jul 27, 2021

In my opinion that particular part should be in long notation, as its "getting started", so it should not use anything fancy.
That said, in line 45 short notation is used for data. That should be changed to data: data for consistency. Can you do that instead, @Yash-Singh1?

@kurkle kurkle merged commit c0d8dd9 into chartjs:master Jul 27, 2021
@LeeLenaleee
Copy link
Collaborator

Maby update title so it will reflect correct changes in the release notes

@Yash-Singh1 Yash-Singh1 changed the title Shorthand labels key in getting started Longhand labels key in getting started Jul 29, 2021
@Yash-Singh1
Copy link
Contributor Author

Done

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

Successfully merging this pull request may close these issues.

None yet

4 participants