On Sat, Aug 13, 2016 at 8:59 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Jeff King <peff@xxxxxxxx> writes: > >> So assuming everything I just said isn't complete bollocks, I think we >> can move to a future where nobody uses the compaction heuristic. And >> there are three ways to deal with that: >> >> 1. The knob and feature stay. It might be useful for somebody who >> wants to experiment in the future. >> >> 2. The knob and feature go away completely. It was an experiment, but >> now we have something more useful. >> >> 3. The feature goes away, but the knob stays as noop, or maybe as an >> alias for the indent heuristic, just because we did ship a version >> that accepts "--compaction-heuristic", and maybe somebody somewhere >> put it in a script? >> >> I think I'd be in favor of (2). > > I am all for (2) [*1*] > I also am in favor of (2). I understand the reasoning for maintaining compatibility, but this was a known experimental feature that was unlikely used by many people. Even if it was, these are the very sorts of people who should be aware that the experimental feature is going away. It reduces code complexity if it just goes away, and I believe the new heuristic is much better (Thank you Michael!!!!!) As for a knob on the new feature, I think it can become the default with a way to disable the feature via command line. I'm not really sure it needs a config option at all. > This and the previous "take a blank line as a hint" are both > heuristics. As long as the resulting code does not tax runtime > performance visibly and improves the resulting output 99% of the > time, there is no reason to leave end-users a knob. "Among 9 hunks > in this patch that touch hello.c, 7 are made much more readable but > 2 are worse" cannot even be helped with a command line option. > Yea I agree. It might be worth having it disabled via the stable patch IDs (? I don't know if we guarantee this?) but otherwise I don't see it being important either way. I would vote for a way to disable it via command line just because we *are* changing behavior here. But I don't think it needs to be a config option at all. > > [Footnote] > > *1* I am also strongly against (3), if only to teach people a > lesson ;-). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html