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

Set a consistent and readable variable name for Array#reduce accumulator variable #7378

Closed
tofumatt opened this issue Jun 19, 2018 · 7 comments
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Code Quality Issues or PRs that relate to code quality [Type] Developer Documentation Documentation for developers

Comments

@tofumatt
Copy link
Member

I pointed out (in #6782 (comment)) that we used settersAcc as the accumulator variable in some code and it wasn't obvious what it was from reading the code.

@aduth pointed out we are quite inconsistent with this variable: #6782 (comment)

It'd be nice to go through and clean these up:

  1. Make them consistent
  2. Put accumulator in the variable name (maybe make that the variable name suffix all the time).
@tofumatt tofumatt added Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Code Quality Issues or PRs that relate to code quality labels Jun 19, 2018
@gziolo gziolo added the Needs Dev Ready for, and needs developer efforts label Apr 10, 2019
@jpjjulie
Copy link

Hey,
Can I please try to work on this issue? I am new to open source and I thought I could contribute to Wordpress to start with. I will try and do my best to fix this issue. I am not sure if I can complete the entire fix but I would like to give it a try.

@lozinska
Copy link
Contributor

lozinska commented Oct 11, 2019

Hi there! I'm new to GitHub and would like to work on this problem, but because I don't have enough experience me and my classmate @jpjjulie decide to work together on this issue and split the work between 2 of us. Thank you

@gziolo
Copy link
Member

gziolo commented Oct 15, 2019

I have just merged #17893 from @lozinska, I will have a look at another PR from @jpjjulie shortly.

@aduth
Copy link
Member

aduth commented Feb 10, 2020

In addition to changes within the current codebase, we should want to consider how to document the coding standard.

Possible locations:

Could make for a good topic of the weekly JavaScript chat.

@ellatrix ellatrix added the [Package] ESLint plugin /packages/eslint-plugin label Aug 20, 2020
@gziolo gziolo added [Type] Developer Documentation Documentation for developers and removed [Package] ESLint plugin /packages/eslint-plugin Needs Dev Ready for, and needs developer efforts labels May 7, 2022
@mburridge
Copy link
Contributor

@gziolo what needs documenting here?

@gziolo
Copy link
Member

gziolo commented Feb 8, 2023

It's all about using good names for variables, it's a very specific use case so I don't know if we need to explicitly document it. Example usage:

return Object.keys( settingValue ).reduce( ( accumulator, key ) => {
if ( ! i18nSchema[ key ] ) {
accumulator[ key ] = settingValue[ key ];
return accumulator;
}
accumulator[ key ] = translateBlockSettingUsingI18nSchema(
i18nSchema[ key ],
settingValue[ key ],
textdomain
);
return accumulator;
}, {} );

In other places, the names can be different, but they still contain the accumulator suffix, example:

const fontSizeAttributeNames = fontSizeNames.reduce(
( fontSizeAttributeNamesAccumulator, fontSizeAttributeName ) => {
fontSizeAttributeNamesAccumulator[
fontSizeAttributeName
] = `custom${ upperFirst( fontSizeAttributeName ) }`;
return fontSizeAttributeNamesAccumulator;
},
{}
);

We have local coding guidelines for Gutenberg at: https://github.com/WordPress/gutenberg/blob/trunk/docs/contributors/code/coding-guidelines.md

General coding standards for JavaScript have a very short section about naming conventions at https://developer.wordpress.org/coding-standards/wordpress-coding-standards/javascript/#naming-conventions.

We could also close the ticket at this point.

@DaisyOlsen
Copy link
Contributor

Thanks for the feedback, @gziolo. We'll close this for now. We can work to improve coding standard guidelines later if there seems to be a need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Code Quality Issues or PRs that relate to code quality [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants