phetsims / perennial

Maintenance tools that won't change with different versions of chipper checked out
MIT License
3 stars 5 forks source link

"update copyright dates from daily grunt work" generates too many clutter commits #260

Closed samreid closed 2 years ago

samreid commented 2 years ago

While working on https://github.com/phetsims/studio/issues/243 I needed to check the joist history, and I saw a distracting amount of clutter commits due to "update copyright dates from daily grunt work".

image

~/apache-document-root/main/joist$ git log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit

* 6cf4bf33 - (HEAD -> master, origin/master, origin/HEAD) update copyright dates from daily grunt work (2 days ago) <phet-dev>
* c1a6957a - EnumerationProperty -> EnumerationDeprecatedProperty, https://github.com/phetsims/phet-core/issues/95 (3 days ago) <zepumph>
* 368c68e6 - grunt update for https://github.com/phetsims/scenery/issues/1333 (3 days ago) <Jonathan Olson>
* 119cc758 - update copyright dates from daily grunt work (7 days ago) <phet-dev>
* 449af71b - Rename Enumeration -> EnumerationDeprecated.js, https://github.com/phetsims/phet-core/issues/95 (8 days ago) <zepumph>
* efa0b690 - Refer to sim as this, see https://github.com/phetsims/phet-io/issues/1838 (9 days ago) <Sam Reid>
* 26233fd5 - update copyright dates from daily grunt work (9 days ago) <phet-dev>
*   9d5ab25a - Merge branch 'master' of https://github.com/phetsims/joist (10 days ago) <Sam Reid>
|\  
| * aee19b61 - labelInnerContent should be optional, https://github.com/phetsims/scenery-phet/issues/717 (10 days ago) <zepumph>
* | d42e7861 - Move updateView to Sim.js, see https://github.com/phetsims/phet-io/issues/1838 (10 days ago) <Sam Reid>
|/  
* 859614af - update copyright dates from daily grunt work (4 weeks ago) <phet-dev>
* 98a773b0 - fix default-import-match-filename in joist, https://github.com/phetsims/chipper/issues/1177 (4 weeks ago) <zepumph>
*

The first step for this issue will be to see if I am the only one bothered by this. If so, then we will close the issue with will-not-fix. If not, we'll come up with a better plan.

Adding to dev meeting agenda to get feedback from other devs, but I'll also ping on slack. I'll also create some "vote" comments below to make it easy to get coarse feedback.

samreid commented 2 years ago

VOTE FOR: This does seem problematic, I think we can find a better way. It probably won't take too long to improve on this.

samreid commented 2 years ago

VOTE FOR: This seems to be rare and easy to look past. It doesn't bother me. Not worth our time to make any changes here.

jessegreenberg commented 2 years ago

In addition to my vote, I also usually use "show history for selection" which excludes changes to the copyright dates. But I recognize that isn't always possible.

samreid commented 2 years ago

@mattpen said:

I’ve never seen these types of commits in files that I edit - I’m going to abstain.

chrisklus commented 2 years ago

I can see how there might be a lot of these around the new year when checking a whole repo, so I'm not opposed to changing it if you'd like to. In WebStorm, I only check history for a single file or by selection within a file, so I never see them cluttering them like this.

samreid commented 2 years ago

Maybe the watch process could update the copyright dates when a file changes, see https://github.com/phetsims/chipper/issues/1173

samreid commented 2 years ago

There were enough votes above that the consensus is:

This seems to be rare and easy to look past. It doesn't bother me. Not worth our time to make any changes here.

Therefore, this doesn't warrant discussion at dev meeting. But I'll leave self assigned based on the preceding comment.

zepumph commented 2 years ago

My personal thought on this was pretty much encapsulated by the thumbs up, but I also want to add that I really like these commits. We used to have these sneak into other commits all the time, or just have outdated copyrights. Now, since calling this in daily grunt work, I haven't thought about it in over a year.

https://github.com/phetsims/chipper/issues/1173 seems likely to be a step backward in my opinion. Would you change the copyright and expect the user to just push that change with their next commit? This seems not ideal, and it could lead to merge conflicts.

My last thought on the topic: As the maintainer of daily grunt work (the process that spams copyright commits to the project). I actually enjoy and appreciate seeing all these commits everywhere I go, because it let's me know that the process is alive an well. Of course, there are other ways that I could get this info, but I guess it comes down to how distracting the commits are.

samreid commented 2 years ago

I feel the appropriate fix for this issue would be a lint rule that is easy to autofix. But it doesn't seem like a significant problem for anyone and therefore should just be closed.