-
Notifications
You must be signed in to change notification settings - Fork 100
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
TODO TDX comments were removed #538
base: main
Are you sure you want to change the base?
Conversation
issue items have been created
Hi - Could you elaborate? Do you mean that these TODOs are all fully resolved, or just that there are now issues tracking each of the TODOs? |
There are issue items created to track all these TODO TDX changes. Some of them have already been resolved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just removing the TODOs removes the context that goes with them.
Instead, please remove only comments that are resolved or no longer need to be tracked. For TODOs that are now tracked with issues, the code could be updated to point to the issue to easily correlate ownership/fix.
@ckotamra all issues originally identified by you are in this repo (I went through them this AM), to expand, rather than delete each TODO comment, please change each TODO comment to either |
Personally I still lean towards keeping the TODOs in the code, even if issues have been made for them, until the issue is resolved. |
TODO TDX comments were removed and corresponding issue items have been created. Closes #523