search

phetsims / chipper

Tools for developing and building PhET interactive simulations.
MIT License
11 stars 14 forks source link

New Lint Rule: Each source file should list at least one author annotation #1414

Closed samreid closed 7 months ago

samreid commented 8 months ago

From a TODO in https://github.com/phetsims/projectile-data-lab/issues/7, we would like to check that each file has at least one author annotation.

samreid commented 8 months ago

I added the lint rule and enabled it for projectile-data-lab. I temporarily enabled the rule project-wide to generate a chip-away report like so:

grunt lint-everything --disable-eslint-cache --chip-away

The result was:

Results from chipAway:

I don't know whether we will want to chip away at this or not. Or maybe just enable on a repo-by-repo basis as desired.

samreid commented 8 months ago

I wrote on slack dev-public:

For Projectile Data Lab, we wanted to make sure each file was annotated with at least one @author annotation. To make sure we didn’t miss anything, we added a new lint rule, described in https://github.com/phetsims/chipper/issues/1414. I also ran the rule across our project to see how our @author coverage was across all our repos. The chip-away report is in https://github.com/phetsims/chipper/issues/1414#issuecomment-1878806624. Should we do anything about this, discuss it in dev meeting, or just enable repo-by-repo (opt in) as desired?

pixelzoom commented 8 months ago

For anyone else wanting to work on fixes... The rule is author-annotation.js, and can be enabled in chipper/eslint/rules/.eslintrc.js

    // Each source file should list at least one author
    'author-annotation': 'off'
pixelzoom commented 8 months ago

My repos were addressed in the above commits.

zepumph commented 8 months ago

After pull we are down to 243 errors here.

samreid commented 8 months ago

Currently at 174 problems remaining across 37 repos. I have done the ones labeled for @samreid, so will self-unassign and I'll add it to the agenda for dev meeting in case we want to discuss further.

marlitas commented 8 months ago

I addressed my repos, unassigning.

pixelzoom commented 8 months ago

At 1/18/24 dev meeting, it was noted that all of the remaining cases are @jonathanolson @jbphet @matthew-blackman , so assigning to them to chip away.

pixelzoom commented 8 months ago

I took care of many of @jbphet's repos, there are still 9 to do.

I discovered that @jonathanolson had apparently addressed most of his repos, but didn't tag this issue in commit, and didn't change those repos off in the list.

pixelzoom commented 8 months ago

I addressed all remaining repos, did a pull-all, and saw no errors with grunt lint-everything --disable-eslint-cache. So I turned the author-annotation rule on.

pixelzoom commented 8 months ago

Slack#developer:

PSA: I addressed all remaining violations of the ‘author-annotation’ lint rule. The rule is now enabled for all repos. Where @author was missing, I used the person who created the file, based on git history.

Slack#DM to @jonathanolson @jbphet @mattpen:

FYI… In localeInfo.js:

IMPORTANT - MUST READ!!!
You may modify this file with new locale information. After modifying the file you must take the following steps:
1. Run ./updateLocaleInfo.js, so that the automatically generated files are also update
2. Notify the responsible developers for rosetta, weddell, yotta, and the website that localeInfo was updated.

For https://github.com/phetsims/chipper/issues/1414, all .js and .ts source files are required to have at least 1 @authorannotation. So in https://github.com/phetsims/chipper/commit/4065e2fe8de8377bd25544992c6ffa19573c1d73, I added an @author annotation to the template in updateLocaleInfo.js, then ran node chipper/js/data/updateLocaleInfo.js which updated localeModuleInfo.js. The comment at the top of localeModuleInfo.js now looks like this:

/**
* This file is automatically generated by js/data/updateLocaleInfo.js. Do not modify it directly.
*
* @author automatically generated by updateLocaleInfo.js
*/
jbphet commented 7 months ago

Thanks @pixelzoom for handling to many of these!