You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
👍 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 code
I'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.
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. :-)
If you want to help out, I'd suggest the following: fix fs error 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 type is junction. In other cases only the second path should be part of the error message.
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 deps or tools that 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.py and sx-pkg-postinstall.sh)
Makefile, Makefile.build, configure
The one for lib in particular will likely need to be broken down further, but this will make it manageable (for the ones other than lib at 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.)
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 src and one for lib and then this can be closed.
Activity
julianduque commentedon Jan 8, 2015
👍 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 codervagg commentedon Jan 9, 2015
I'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.
btd commentedon Jan 9, 2015
It could be more easy to automate it in jenkins (as build containers for iojs on jenkins) with task scanner?
vkurchatkin commentedon Jan 9, 2015
I was talking about one time thing, not about actual hook.
Indeed, but comments can be rather vague or obsolete, so real github issues are required.
bnoordhuis commentedon Jan 9, 2015
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. :-)
caitp commentedon Jan 9, 2015
I'm bored, I'll propose a change for the most trivial of these that I can find :)
rvagg commentedon Jan 9, 2015
haha, very well done @caitp, next!
piscisaureus commentedon Jan 11, 2015
@caitp
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:
typeisjunction. In other cases only the second path should be part of the error message.caitp commentedon Jan 11, 2015
sure, I'll create an issue with an idea I've got there
piscisaureus commentedon Jan 11, 2015
@caitp See also #207
mrjoelkemp commentedon Feb 14, 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.
mrjoelkemp commentedon Feb 14, 2015
Nevermind. I see they've been fixed
Qard commentedon Aug 14, 2015
I wonder if we should consider making future TODO/FIXME comments include issue numbers?
Trott commentedon Jan 9, 2016
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:I'd like to close this issue and open issues to replace it focusing on particular areas, one each for:
cpplint.pyandsx-pkg-postinstall.sh)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.)
vkurchatkin commentedon Jan 9, 2016
@Trott let's do this!
bnoordhuis commentedon Jan 9, 2016
@Trott You can drop cpplint.py from the list, it's mostly not written by us.
thefourtheye commentedon Jan 10, 2016
Actually in few places people's names are also used.
Trott commentedon Jan 12, 2016
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.Trott commentedon Jan 12, 2016
OK, separate issues opened. Some of them may yet still need to be broken down further, but it's a start. Closing this one.