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

TODOs without an associated GitHub issue #28

Closed
8 tasks done
pixelzoom opened this issue Jan 15, 2023 · 3 comments
Closed
8 tasks done

TODOs without an associated GitHub issue #28

pixelzoom opened this issue Jan 15, 2023 · 3 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 15, 2023

In Slack#pistachio on 1/12/23, @zepumph said:

Since there are so many people working for number-play and associated repos, I would like to add the lint rule where we require issues for todos. There are only 5 failures currently. Do we like that idea to help keep things organized?

There were certainly more than 5 TODOs for "for number-play and associated repos" that do not have an associated GitHub issue. It looks like someone (@zepumph?) turned on the lint rule. That likely resulted in phetsims/number-compare#11, which I've addressed.

Searching the 3 "number" repos, I still see many TODOS (and variations of TODO) that are not failing lint, and do not have an associated GitHub issues. Is there a problem with the lint rule? Is there a problem with how the lint rule is set up for these repos?

Starting with @zepumph.


  • Subitizer.ts
215         // TODO-DESIGN: Is increasing this still desirable now that you can control it easily?
  • CountingGameLevelNode.ts:
56 // TODO: The parts of this file that are used for the play area node need to be refactored once the play area is updated.
  • CountingPlayAreaNode.ts:
99    // TODO-TS: Get rid of this binding pattern. Update function signatures in the attributes.
271    // TODO: This seems like a weird sidestep to try tenframes first and maybe be moved
337    // TODO: Docs and cleanup
367    * TODO
381    * TODO
  • MissingVoiceWarningButton.ts:
20 // TODO: Factor out with other buttons
  • CardCreatorNode.ts:
26 // TODO: add comments
  • LabScreenView.ts:
70     // TODO: make file constants?
208    // TODO: rename to addTenFrameNode and removeTenFrameNode
  • SymbolCardNode.ts:
16 // TODO: This should be in its own file, not exported as a type here.
  • TenFrameCreatorPanel.ts
41      // TODO: Factor out with CountingObjectCreatorPanel
@pixelzoom
Copy link
Contributor Author

Let's please not create a separate issue for each of these TODOs, as has been done elsewhere. We don't need a pile of new issues to sort through, as we're trying to reduce the number of issues so we can publish. One issue for all TODOs that need to be addressed would be fine, and we can create issues if any of them require an issue.

@pixelzoom
Copy link
Contributor Author

BlockValuesNodes.ts

    // TODO: Fix drawing so initial state can be setup and updated here

pixelzoom added a commit that referenced this issue Jan 15, 2023
pixelzoom added a commit to phetsims/number-compare that referenced this issue Jan 15, 2023
pixelzoom added a commit that referenced this issue Jan 15, 2023
pixelzoom added a commit to phetsims/number-play that referenced this issue Jan 15, 2023
@pixelzoom
Copy link
Contributor Author

I created #29, associated it with all misc TODOs, and closed some of the individual GitHub issues that @zepumph and I had created.

I still don't understand how the lint rule is running (I don't see it in any of the relevant package.json's) or why it's identifying some TODOs but not others. I'll create a separate issue to fix that, so that we do not inadvertantly create new TODOs without issues.

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