Change all XXXs to FIXMEs #3303
Comments
|
Good idea. With suitable advances in robot technology, the robot could also fix the bugs ;-) |
|
Nominating -- I think it would be great to have this before "maturity #5 - production ready", and then we would also not need both XXXs and FIXMEs. |
|
Would still be handy |
|
accepted for backwards-compatible milestone |
|
Triage: loads of XXXs, and @bors is still only merging the bug fixes. |
|
To be clear, this is basically 'open an issue for every XXX, put text in the issue, switch to FIXME with a number'? I'm willing to do that, it's less than two hundred of them. |
|
@steveklabnik: Correct. |
|
@steveklabnik That would be great. In the process, you'll probably discover some that are already fixed or don't make sense; feel free to remove them completely or ask the person listed in |
|
I would like to work on it! |
|
@huonw Since it seems this has been left alone, I'd like to tackle this, but I have a question (really new here): are these issues meant to be created manually? Some of the descriptions next to the XXX comments are not descriptive enough to stand on their own. |
|
if there isn't much information in the XXX, there isn't much to put in the On Sat, Jan 25, 2014 at 1:50 PM, salemtalha notifications@github.comwrote:
|
|
@cmr Could you clarify your last statement a bit? Not sure I follow. |
|
leave, not live. replace the XXX with FIXME #XXXXX, but leave the On Sat, Jan 25, 2014 at 2:01 PM, salemtalha notifications@github.comwrote:
|
|
@cmr So if I understand correctly: For all instances of "// XXX: comment", if the comment is explanatory enough then create an issue with the comment included and change the source code to "// FIXME #12345: comment". But if the comment doesn't make much sense, should I literally replace it with "// FIXME #XXX: comment"? I read the tidy script and I believe it rejects FIXMEs without numeric issue numbers. |
|
@brson I came up with a quick script that maybe does what you're meaning, what do you think? #!/usr/bin/env python
import sys, fileinput, subprocess, re, requests, json
def report_error_name_no(name, no, s):
global err
print("%s:%d: %s" % (name, no, s))
err=1
def report_err(s):
report_error_name_no(fileinput.filename(), fileinput.filelineno(), s)
def report_warn(s):
print("%s:%d: %s" % (fileinput.filename(), fileinput.filelineno(), s))
file_names = [s for s in sys.argv[1:] if (not s.endswith("_gen.rs"))
and (not ".#" in s)]
try:
for line in fileinput.input(file_names, inplace=1):
if "// FIXME" in line:
comment = line.partition(':')[2]
url = 'https://api.github.com/repos/mozilla/rust/issues'
payload = {
'title' : comment,
'body' : ''
}
headers = { 'content-type' : 'application/json' }
r = requests.post(url, data=json.dumps(payload), headers=headers)
if r.status_code == 201:
sys.stdout.write(line.replace("FIXME", "FIXME(#" + r['number'] + ")")
else:
sys.stdout.write(line)
else:
sys.stdout.write(line)
except UnicodeDecodeError, e:
report_err("UTF-8 decoding error " + str(e))
sys.exit(err) |
|
@salemtalha Yes, that is something like I was thinking. We would probably want to do the same for |
|
Oh, I don't think this script idea will work. Looking through the code there are lots of places where many XXX's refer to the same issue, like "XXX: bad copy". Personally, I just don't think it's a good policy to require issues for all FIXMEs. There are a lot of them and many are so minor it doesn't really matter if they ever get fixed. |
|
Perhaps a smart script could coalesce FIXMEs with the same description. |
|
Per #11815 my preference is to remove our futile attempts to enforce the FIXME policy with the tidy script, replace all the XXXs with FIXMEs, then keep encouraging people to file issues for FIXMEs manually. |
|
Sounds good. I'll get on this. |
While I agree, don't underestimate the power of "I wrote something that closed an issue forreal" for new contributors. If they're so minor, they should be easy for a new contributor to handle, and making them 'second-class' makes the contribution feel second class. |
|
I run |
|
Sorry, I messed up with my local repo at first so here are two references. Now it must be fine. |
Few places where previous version of tidy script cannot find XXX: * inside one-line comment preceding by a few spaces; * inside multiline comments (now it finds it if multiline comment starts on the same line with XXX). Change occurences of XXX found by new tidy script.
|
Can this issue be closed? I think there is nothing to do more here. |
|
Yes, thanks @JIghtuse |
Our development policy requires marking all FIXME's with issue numbers, and this is enforced by the tidy script. Having issues for all the FIXME's is particularly useful since they often consist of small tasks you can do at leisure or that new contributors can pick off easily.
Alas, it is awkward to open issues for bugs that one is still in the process of creating, and I have started marking problems with XXX.
With suitable advances in robot technology we could streamline the process: human write unnumbered FIXMEs; robot periodically find them, parse descriptions, open issues on github, patch issue numbers into code and submit a pull request.
To support the patching we would probably require FIXME to be written something like
FIXME (####), so that the patched version is guaranteed to be under the line length limit.The text was updated successfully, but these errors were encountered: