Skip to content

Conversation

@nazar-lazarchuk
Copy link
Contributor

@nazar-lazarchuk nazar-lazarchuk commented Jan 23, 2022

To me, not a native English speaker, the name of the variable counter looks a little better in the "Stores" section because it keeps the value of the counter (not the "count" function, as I thought when I first read documentation).

I also think it's better to use camelCase instead of snake_case for variables, so I have replaced count_value to counterValue in some lines.
snake_case code style is used in a few more places, so in my opinion, it would be good to correct it as well.

I will be happy to discuss this pull request

@benmccann
Copy link
Member

changing from snake_case to camelCase definitely seems like a good change to me

i'm ambivalent on the variable name. either one seems fine to me

I think changing the text from "The count is" to "The counter is" doesn't read as well

@nazar-lazarchuk
Copy link
Contributor Author

nazar-lazarchuk commented Jan 26, 2022

changing from snake_case to camelCase definitely seems like a good change to me

i'm ambivalent on the variable name. either one seems fine to me

I think changing the text from "The count is" to "The counter is" doesn't read as well

Okay, then I think it's better to do revert count, but keep camelCase here and in another docs sections.
What do you think?

@benmccann
Copy link
Member

Sounds fine to me

@Rich-Harris
Copy link
Member

Yep, I would prefer we keep the variable as count. The store should be named for the thing itself — you'd have $theme rather than $themer and $enabled rather than $enabler, and this is no different. 'count' here is being used as a noun rather than a verb

@nazar-lazarchuk nazar-lazarchuk changed the title [docs] Rename a variable count to counter in the Stores section [docs] Rename a variable count_value to countValue in the Stores section Jan 26, 2022
@nazar-lazarchuk
Copy link
Contributor Author

nazar-lazarchuk commented Jan 26, 2022

Guys, so I left only renaming count_value to countValue.
If you don't mind I will rename snake_case to camelCase in other docs in the new PR's

@dummdidumm dummdidumm merged commit e460acc into sveltejs:master Jan 27, 2022
nevilm-lt pushed a commit to nevilm-lt/svelte that referenced this pull request Mar 14, 2022
nevilm-lt pushed a commit to nevilm-lt/svelte that referenced this pull request Apr 21, 2022
nevilm-lt pushed a commit to nevilm-lt/svelte that referenced this pull request Apr 21, 2022
nevilm-lt pushed a commit to nevilm-lt/svelte that referenced this pull request Apr 22, 2022
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.

4 participants