Stream: t-compiler/wg-nll

Topic: #19546 Q: should we reject FIXME without issue numbers?


davidtwco (Dec 17 2018 at 13:36, on Zulip):

It might be worth trying to not accept new code that has a // FIXME or // TODO that doesn't also reference a issue number so it is easier to follow up on whether or not that issue was resolved and the comment can be removed.

pnkfelix (Dec 17 2018 at 13:39, on Zulip):

We used to do this, maybe we still do for one of those

pnkfelix (Dec 17 2018 at 13:39, on Zulip):

i.e. tidy used to reject // FIXME

pnkfelix (Dec 17 2018 at 13:39, on Zulip):

and years ago one of the team members went through adding issue numbers to every fixme

pnkfelix (Dec 17 2018 at 13:39, on Zulip):

until someone stopped them and said "this is not helping."

davidtwco (Dec 17 2018 at 13:40, on Zulip):

Huh, I wonder what the reasoning behind that was.

pnkfelix (Dec 17 2018 at 13:40, on Zulip):

(I personally stand more on the side of "file issues and ask questions later")

pnkfelix (Dec 17 2018 at 13:40, on Zulip):

let me see if I can find the context, maybe that will present the matter in a more favorable light

davidtwco (Dec 17 2018 at 13:42, on Zulip):

@pnkfelix #11815?

pnkfelix (Dec 17 2018 at 13:42, on Zulip):

Issues that I can find quickly that provide context: #3303 #11815 #19546

davidtwco (Dec 17 2018 at 13:43, on Zulip):

Huh, from that it seems like people ended up liking it after it was gone but the ship has sailed.

pnkfelix (Dec 17 2018 at 13:43, on Zulip):

ah maybe I misremembered, and what was "thumbs downed" was having a robot automatically replace // XXX with // FIXME(fresh issue number)

pnkfelix (Dec 17 2018 at 13:45, on Zulip):

maybe we could bring it (the requiring issue number on FIXME) back, perhaps not project wide, but on a per-directory level

pnkfelix (Dec 17 2018 at 13:46, on Zulip):

and that way teams/working groups would have agency to decide for themselves which policy they prefer to have enforced by tidy.

pnkfelix (Dec 17 2018 at 13:46, on Zulip):

(I'm going to rename this fork of the original topic now)

nikomatsakis (Dec 17 2018 at 17:23, on Zulip):

It's an awkward policy at best

nikomatsakis (Dec 17 2018 at 17:23, on Zulip):

I found it useful in that I like to be able to leave notes that I know won't get comitted (though you can use // TODO) for that

nikomatsakis (Dec 17 2018 at 17:24, on Zulip):

I don't like converting existing FIXMEs into issues

nikomatsakis (Dec 17 2018 at 17:24, on Zulip):

99.9% of them don't deserve to be issues

nikomatsakis (Dec 17 2018 at 17:24, on Zulip):

but I don't mind not permitting the text FIXME to be committed, either -- that kind of forces the question. Do you really think is so important it's worth filing an issue? Or would you rather just change it to a regular comment? :)

nikomatsakis (Dec 17 2018 at 17:24, on Zulip):

I'm not 100% sure why we changed the policy back

nikomatsakis (Dec 17 2018 at 17:25, on Zulip):

I found it useful in that I like to be able to leave notes that I know won't get comitted (though you can use // TODO) for that

I'll note that the ./x.py tidy workflow isn't ideal here, either

nikomatsakis (Dec 17 2018 at 17:25, on Zulip):

I think in my ideal world it wouldn't enforce the FIXME rule locally

nikomatsakis (Dec 17 2018 at 17:25, on Zulip):

only on the bots

nikomatsakis (Dec 17 2018 at 17:25, on Zulip):

or else maybe there'd be a way to do it as the final step or something

nikomatsakis (Dec 17 2018 at 17:25, on Zulip):

as I like to be able to run tests before the final commit in the PR series

pnkfelix (Dec 18 2018 at 10:18, on Zulip):

why can't you run tests without running tidy when you do your local testing?

pnkfelix (Dec 18 2018 at 10:19, on Zulip):

I mean, I know that I have driver shell scripts that do a sequence like "tidy; build; test"; but it seems easy to omit tidy when that's what you want?

nikomatsakis (Dec 18 2018 at 15:14, on Zulip):

why can't you run tests without running tidy when you do your local testing?

it's just that ./x.py test runs tidy

nikomatsakis (Dec 18 2018 at 15:15, on Zulip):

maybe there is even a --no-tidy option, I don't know

Last update: Nov 21 2019 at 15:05UTC