salesforce / lwc

⚡️ LWC - A Blazing Fast, Enterprise-Grade Web Components Foundation
https://lwc.dev
Other
1.65k stars 394 forks source link

Validate if work is needed for closed TODO issues #4061

Open wjhsf opened 8 months ago

wjhsf commented 8 months ago

Currently, we have 33 // TODO* comments in the codebase that reference 21 issues that have been closed. We should re-visit those comments to assess whether the work has been done, or, if not, whether it is unblocked and/or still something we want to do.

To generate the list of closed issues:

# List all files tracked by git on the master branch
git ls-tree --name-only master -r |\
# Find all TODO [#0000] comments
xargs grep -o 'TODO \[#.*\]' |\
# Extract the issue number from the grep result
sed 's/.*TODO \[#\(.*\)\]/\1/' |\
# Get unique issue numbers
sort | uniq |\
# Query GitHub and print the issue number if the issue is closed
xargs -I% gh issue view --json number,closed -q 'select(.closed)|.number' % |\
# Print to console and save to file
tee closed-todo-issues.txt

Technically one is a `/ TODO`, so keep an eye out for that if you're wielding the regex hammer! Also, issue #1234 is a false positive.

wjhsf commented 8 months ago

I've pushed a branch, todone, which adds a comment // TODO: Is this TODOne? before all of the TODO comments with currently closed issues.

If you don't want to search the codebase yourself, you can run yarn lint and eslint will tell you exactly where those comments are.

shiladityab24 commented 8 months ago

@wjhsf

yarn run v1.22.19
$ eslint packages/ scripts/ --ext=js,mjs,ts,only,skip

<--- Last few GCs --->

[9373:0x6aa8000]   409387 ms: Scavenge 2028.4 (2078.6) -> 2022.9 (2080.1) MB, 5.39 / 0.01 ms  (average mu = 0.785, current mu = 0.452) allocation failure;
[9373:0x6aa8000]   409828 ms: Mark-Compact 2032.4 (2087.8) -> 2008.1 (2080.2) MB, 403.45 / 0.01 ms  (average mu = 0.667, current mu = 0.379) allocation failure; scavenge might not succeed
[9373:0x6aa8000]   409930 ms: Scavenge 2019.4 (2083.3) -> 2012.1 (2098.1) MB, 7.06 / 0.00 ms  (average mu = 0.667, current mu = 0.379) allocation failure;

<--- JS stacktrace --->

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
----- Native stack trace -----

 1: 0xca5580 node::Abort() [/home/shiladitya/.nvm/versions/node/v20.11.1/bin/node]
 2: 0xb781f9  [/home/shiladitya/.nvm/versions/node/v20.11.1/bin/node]
 3: 0xeca4d0 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, v8::OOMDetails const&) [/home/shiladitya/.nvm/versions/node/v20.11.1/bin/node]
 4: 0xeca7b7 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, v8::OOMDetails const&) [/home/shiladitya/.nvm/versions/node/v20.11.1/bin/node]
 5: 0x10dc505  [/home/shiladitya/.nvm/versions/node/v20.11.1/bin/node]
 6: 0x10f4388 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/home/shiladitya/.nvm/versions/node/v20.11.1/bin/node]
 7: 0x10ca4a1 v8::internal::HeapAllocator::AllocateRawWithLightRetrySlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/home/shiladitya/.nvm/versions/node/v20.11.1/bin/node]
 8: 0x10cb635 v8::internal::HeapAllocator::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/home/shiladitya/.nvm/versions/node/v20.11.1/bin/node]
 9: 0x10a8c86 v8::internal::Factory::NewFillerObject(int, v8::internal::AllocationAlignment, v8::internal::AllocationType, v8::internal::AllocationOrigin) [/home/shiladitya/.nvm/versions/node/v20.11.1/bin/node]
10: 0x1503a16 v8::internal::Runtime_AllocateInYoungGeneration(int, unsigned long*, v8::internal::Isolate*) [/home/shiladitya/.nvm/versions/node/v20.11.1/bin/node]
11: 0x7f2a3f699ef6
Aborted
error Command failed with exit code 134.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

This list was generated from the todone branch using yarn lint command , is this the end of the list or there are more , please correct If I am wrong.

wjhsf commented 8 months ago

That's an error output; running eslint caused the node process to crash. I'm not sure why that would happen; you may want to re-install your dependencies and try again: yarn install && yarn run lint.

In any case, it turns out that I had not actually committed the "TODOne" comments before, but now I have! 🚀

wjhsf commented 7 months ago

Here's a list of all the current TODOs in published packages.