Repository navigation
Review TODOs and FIXMEs #264
Description
Activity
👍 I like this approach, would be ideal to leave the TODO or FIXME comment associated with an issue in GitHub as proposed with
github-todos, it's sometimes easier to fix or contribute to an issue if you exactly where it is on the codeI'm not sure about automating this, might be a bit too noisy, but TODOs and FIXMEs are a great place for new contributors to get started and issues could be labelled as such.
It could be more easy to automate it in jenkins (as build containers for iojs on jenkins) with task scanner?
unnecessary a bit, because those comments should be created by some of committers from core team, such that i think those should be personal work rather than community actually, issues or PRs would also absolutely depends on someone who created the TODO :(
I was talking about one time thing, not about actual hook.
TODOs and FIXMEs are a great place for new contributors to get started and issues could be labelled as such.
Indeed, but comments can be rather vague or obsolete, so real github issues are required.
haha @vkurchatkin you are right, i'm confused by Rod said automation :)
I suspect that a sizable fraction of TODOs and FIXMEs are stale or not actionable.
I know I'm guilty of putting TODOs in my code that assume that once we have $mythical_feature_x, we'll finally be able to do this right! I'm probably not the only one who does. :-)
I'm bored, I'll propose a change for the most trivial of these that I can find :)
haha, very well done @caitp, next!
If you want to help out, I'd suggest the following: fix
fserror messages.Currently if
fs.rename('foo', 'bar')fails, an error gets printed that looks like this:
Error: ENOENT, rename 'C:\Users\Bert Belder\foo'
The error includes only the first or the second path, depending on what the error was. That makes very little sense. Instead, what I'd like to see is:
Error: ENOENT, rename 'C:\Users\Bert Belder\foo' -> 'C:\Users\Bert Belder\bar'The same also applies to:
- fs.link(source, target);
- fs.symlink(dest, path, type), but only on windows when
typeisjunction. In other cases only the second path should be part of the error message.
sure, I'll create an issue with an idea I've got there
- addedhelp wantedIssues that need assistance from volunteers or PRs that need help to proceed.Issues that need assistance from volunteers or PRs that need help to proceed.
on Jan 30, 2015 @piscisaureus @caitp is the error message issue still unresolved? I'd like to give it a shot.
I'd also love more beginner-friendly tickets being labeled with help-wanted.
Nevermind. I see they've been fixed
- addedgood first issueIssues that are suitable for first-time contributors.Issues that are suitable for first-time contributors.
on May 28, 2015 I wonder if we should consider making future TODO/FIXME comments include issue numbers?
This issue is a year old and has seen no recent activity.
There might be some imperfections in my methods, but when I search through the current master branch and remove obvious false positives (markdown files, things in
depsortoolsthat we don't control, etc.), here's what I get:Path TODOs,FIXMEs,XXXs configure 2 lib 61 Makefile 1 Makefile.build 1 src 44 test 36 tools 12 I'd like to close this issue and open issues to replace it focusing on particular areas, one each for:
- lib
- src
- test
- tools (just
cpplint.pyandsx-pkg-postinstall.sh) - Makefile, Makefile.build, configure
The one for
libin particular will likely need to be broken down further, but this will make it manageable (for the ones other thanlibat least) to keep a task list of existing TODO (etc.) items in each area.I'll wait at least a couple days in case anyone has a strong objection to this approach. (I'm guessing some people will think it's a pointless exercise, but I'm optimistic that no one will think that it's actively harmful.)
@Trott let's do this!
@Trott You can drop cpplint.py from the list, it's mostly not written by us.
Actually in few places people's names are also used.
Wow, I had a lot of false positives in the test directory. Turns out there were only 11 in there. Still have to open a PR for
srcand one forliband then this can be closed.OK, separate issues opened. Some of them may yet still need to be broken down further, but it's a start. Closing this one.
Thoughts?