Skip to content

JS: Clean up some TODO comments#18451

Merged
asgerf merged 14 commits into
github:js/shared-dataflow-branchfrom
asgerf:jss/cleanup-todos
Jan 9, 2025
Merged

JS: Clean up some TODO comments#18451
asgerf merged 14 commits into
github:js/shared-dataflow-branchfrom
asgerf:jss/cleanup-todos

Conversation

@asgerf

@asgerf asgerf commented Jan 9, 2025

Copy link
Copy Markdown
Contributor

All TODO comments have corresponding issues internally.

There are a few TODOs that I left in when in cases where they add clarification and represent a relatively local task close to where the comment appears. For larger tasks, issues are more appropriate.

asgerf added 14 commits January 9, 2025 09:19
This isn't going to become a taint step, the workaround is the permanent solution
The test case actually has the correct result now
There is an issue for tracking this. It's not a small fix.
This requires changes to the shared data flow library, not something we should track with a TODO in the JS codebase
This is useful info, but not something that can be fixed locally in this query, so a TODO comment isn't helping
We have an issue for fixing the underlying problem
The RHS of an assignment actually has a post-update node now
It is not subsumed by the other case, both cases are needed
@github-actions github-actions Bot added the JS label Jan 9, 2025
@asgerf asgerf added the no-change-note-required This PR does not need a change note label Jan 9, 2025
@asgerf asgerf marked this pull request as ready for review January 9, 2025 10:40
Copilot AI review requested due to automatic review settings January 9, 2025 10:40
@asgerf asgerf requested a review from a team as a code owner January 9, 2025 10:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again, by re-requesting a review.

@asgerf asgerf merged commit 0f6e8bf into github:js/shared-dataflow-branch Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

JS no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants