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

One issue to hold every TODO in the project #1129

Open
zepumph opened this issue Sep 13, 2023 · 4 comments
Open

One issue to hold every TODO in the project #1129

zepumph opened this issue Sep 13, 2023 · 4 comments

Comments

@zepumph
Copy link
Member

zepumph commented Sep 13, 2023

From #1128, it will be a good starting point if every TODO that doesn't currently have an issue assigned to it, points to this issue instead. Then we will get the benefits of having closed issues reopened when TODOs point there for all repos.

@zepumph
Copy link
Member Author

zepumph commented Sep 13, 2023

There are currently 890 TODOs pointing to this issue. Fun!

pixelzoom added a commit to phetsims/xray-diffraction that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/xray-diffraction that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/xray-diffraction that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/shred that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/shred that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/shred that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/shred that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/aqua that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/normal-modes that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/normal-modes that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/john-travoltage that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/john-travoltage that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/john-travoltage that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/least-squares-regression that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/optics-lab that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/optics-lab that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/rutherford-scattering that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/rutherford-scattering that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/sound-waves that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/color-vision that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/color-vision that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/area-builder that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/area-builder that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/area-builder that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/faradays-law that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/build-an-atom that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/capacitor-lab-basics that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/charges-and-fields that referenced this issue Sep 14, 2023
pixelzoom added a commit to phetsims/coulombs-law that referenced this issue Sep 14, 2023
@pixelzoom
Copy link
Contributor

pixelzoom commented Sep 14, 2023

We discussed this issue in dev meeting. Our previous decision to require TODOs for issues in common-code and published sims is not being followed and is not supported by all devs. And I don't know what the consensus was -- it seems like it's up to each dev, and I think that's problematic.

We did agree that no new references to this issue should be created.

I went a step further, and did some work to eliminate references to this issue. Here's a summary:

There were a number of places where a link to this issue was errorneously added, because the issue was not on the same line as "TODO". In those cases, I simply removed the reference to those issues. For example in MagnetRegionManager:

    // TODO: The code below looks wrong.  The return type doesn't match the header docs, and the whole thing is too https://github.com/phetsims/tasks/issues/1129
    //  tightly coupled to the way bounds are managed in the model.  See
    // https://github.com/phetsims/faradays-law/issues/164. Also, it seems like it should return null instead of -1 if
    // no intersection is found.

In these repos, there were a small number of TODOs, so I created specific GitHub issues:

  • aqua
  • balloons-and-static-electricity
  • color-vision
  • faradays-law
  • inverse-square-law-common
  • least-squares-regression
  • normal-modes
  • optics-lab
  • rutherford-scattering
  • shred
  • xray-diffusion

In these repos, I created one repo-specific "Address TODOs" issue:

  • area-builder
  • balancing-act
  • build-an-atom
  • bending-light
  • capacitor-lab-basics
  • charges-and-fields
  • coulombs-law
  • sound-waves

Sim repos that continue to point to this issue:

  • cck-black-box-study (23)
  • cck-common (9)
  • density-buoyancy-common (27)
  • eating-and-exercise-energy (8)
  • estimation (2)
  • fluid-pressure-and-flow (30)
  • forces-and-motion-basics (32)
  • fraction-comparison (5)
  • molecule-shapes (12)
  • phet-io-test-sim (6)
  • interaction-dashboard (5)

Common-code repos that continue to point to this issue. dot, kite, and scenery are the obvious problems here.

@pixelzoom
Copy link
Contributor

Back to @zepumph to continue work on this issue.

Regarding the errorneous TODOs identified by the lint rule... In Slack#DM, @zepumph said:

Yes I think we could adapt our rule to support that. Could you make an issue in chipper please? It is because eslint thinks of a single "comment" only as one line comment or one block comment. It doesn't combine those. I hadn't seen that case before but obviously we want to support that (I certainly write TODOs like that).

@zepumph
Copy link
Member Author

zepumph commented Sep 15, 2023

After the above work, there are 189 TODOs pointing to this issue. That is better than 890! @pixelzoom did a good job of outlining where they live right now. I'm going to unassign for now. Thanks all.

@zepumph zepumph removed their assignment Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants