Site-www: Create issues for TODO comments

Created on 19 Nov 2019 · 20Comments · Source: dart-lang/site-www

As of today, there are 39 occurrences of TODO in site-www source code. Some of them are valid (e.g. the Todo app), but many are actual TODOs and should be tracked by GitHub issues.

We should (1) add issues for the actual TODOs and (2) either remove the corresponding TODO from the source code or put the issue number by the TODO comment.

beginner cleanurefactoring e1-hours help wanted p2-medium

Source

kwalrath

👍2

All 20 comments

i'd like to work on this @kwalrath

lenniezelk on 20 Nov 2019

🎉1

@lenniezelk it's yours. Thank you!

kwalrath on 20 Nov 2019

🎉1

@kwalrath i found 36 valid TODOs. Do i make the 36 issues?

lenniezelk on 25 Nov 2019

@kwalrath i found 36 valid TODOs. Do i make the 36 issues?

Good question. First I'd make sure the TODOs don't already have an associated issue. (If they do, we should update the source code to refer to that issue... so you could either do that or add a note to the issue that the TODO needs to be updated.)

Second, it's fine to combine related TODOs in a single issue if (1) they're likely/easy to be fixed in a single PR or (2) they have a common theme that's likely to require other files (that don't have TODOs) to be affected.

So probably you'll file >1 issue but < 36. Make sense?

kwalrath on 25 Nov 2019

I have tried grouping the ones that seem related. Any more that can be grouped or what do you think needs to change? I found two that possibly have issues no.6 and no.20 below.
https://gist.github.com/lenniezelk/ba4526665f321cde5aa8aa634ef61724
@kwalrath

lenniezelk on 2 Dec 2019

I haven't had time to look in depth at your groupings, but I aim to give you feedback ~today or tomorrow~Thursday or Friday. Thanks very much for your work on this!

kwalrath on 3 Dec 2019

Finally looked at https://gist.github.com/lenniezelk/ba4526665f321cde5aa8aa634ef61724. Thanks so much for your work on this! Your groupings look good. Some specific comments:

5 (TODO: refactor into an AbstractLlamaGreetingFactory?) is a joke comment intended to be shown in the docs, so no issue is necessary.
We might be able to combine some TODOs under an issue related to infrastructure tools: 4, 6, 7, 8, ... (maybe that's all?).
12 might be the same as #1744 (although it should have a new comment pointing to the TODO).
Agreed that 20 seems to be covered by #2010.

Again, thanks!!

kwalrath on 9 Dec 2019

Hello @kwalrath!
I was curious if there is anything else that needs to be done regarding this issue.

srLitem on 18 Dec 2020

@srLitem thanks for asking! I suspect we do still need to do something, especially since we now have 46 occurrences of TODO in code (up from 39). Are you willing to do a review similar to the one @lenniezelk did?

kwalrath on 5 Jan 2021

@kwalrath, as @srLitem didn't reply, may I work on this TODO issue.

woinbo on 8 Jan 2021

Go ahead, @woinbo. I suspect @srLitem needed a response sooner than I was able to give it. :(

kwalrath on 9 Jan 2021

❤1

Go ahead, @woinbo. I suspect @srLitem needed a response sooner than I was able to give it. :(

Thank you @kwalrath, for assigning it to me.

woinbo on 9 Jan 2021

I have grouped the remaining TODOs in this file https://gist.github.com/woinbo/f71447550872339e51683f50bfc2d464 .
Please check it out! Anything else for me.
@kwalrath

woinbo on 9 Jan 2021

I have grouped the remaining TODOs in this file https://gist.github.com/woinbo/f71447550872339e51683f50bfc2d464 .
Please check it out! Anything else for me.
@kwalrath

@kwalrath can you check this one too..

woinbo on 12 Jan 2021

Your groupings look great, @woinbo. Could you please file the issues and then create one or more PRs to add the relevant issue URL/number next to each TODO in the source code? Thank you!

kwalrath on 12 Jan 2021

Hello @kwalrath, changes are to be done in the grouping (gist) file or in the main site-www source code. Thank you!

woinbo on 12 Jan 2021

The gist is just a planning doc. We need issues on the site-www tracker, and references to those issues in the site-www source code.

kwalrath on 12 Jan 2021

ok, sure I will do it. Thank you @kwalrath, could you also review my PR #2841

woinbo on 12 Jan 2021

👍1

Hey @kwalrath, I did create issue #2852 for all the TODO' and list the files. can you please check and if anything needs to be changed do inform me.

woinbo on 13 Jan 2021

Your groupings look great, @woinbo. Could you please file the issues and then create one or more PRs to add the relevant issue URL/number next to each TODO in the source code? Thank you!

PR updating TODO's with issues number next to each TODO