<div dir="ltr"><div>Hi,</div><div><br></div><div>Thanks for positioning some arguments with respect to upstream development, appreciate it!</div><div><br></div><div>If `git blame` is what you're after, then it seems like we'd bundle refactoring per "region of code" (function?) rather than multiple files together: changes across files shouldn't create a long sequence of `git blame` tracking right?</div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Feb 20, 2020 at 10:33 AM Robinson, Paul <<a href="mailto:paul.robinson@sony.com">paul.robinson@sony.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<div lang="EN-US">
<div class="gmail-m_-2898440583235044390WordSection1">
<p class="MsoNormal">Hi Mehdi!<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">I think the value to upstream (of doing mass reformatting in fewer commits) has to do with the intrusion of nonfunctional commits into `git blame` kinds of research. Every line that someone touches for a formatting reason necessarily obscures
the history of functional changes in that block of code. The fewer of those that people have to work around, the better. I admit this is a small concern, but it wasn’t so long ago that I was looking back at some history and had to wade through way more formatting
commits than functional commits to get to the log message that told me what I needed to know.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Some formatting changes just come with the territory, but if I have to deal with 1 or 2 instead of 5 or 8, it does improve my day.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">HTH,<u></u><u></u></p>
<p class="MsoNormal">--paulr<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<div style="border-style:none none none solid;border-left-width:1.5pt;border-left-color:blue;padding:0in 0in 0in 4pt">
<div>
<div style="border-style:solid none none;border-top-width:1pt;border-top-color:rgb(225,225,225);padding:3pt 0in 0in">
<p class="MsoNormal"><b>From:</b> Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>> <br>
<b>Sent:</b> Thursday, February 20, 2020 1:16 PM<br>
<b>To:</b> Robinson, Paul <<a href="mailto:paul.robinson@sony.com" target="_blank">paul.robinson@sony.com</a>><br>
<b>Cc:</b> Philip Reames <<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>>; Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>>; <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>; Valentin Churavy <<a href="mailto:v.churavy@gmail.com" target="_blank">v.churavy@gmail.com</a>><br>
<b>Subject:</b> Re: [llvm-dev] amount of camelCase refactoring causing some downstream overhead<u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Wed, Feb 19, 2020 at 12:10 PM Robinson, Paul via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal">Hi Philip,<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">I think you might be reading more into the suggestion/discussion than is actually there.<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<ul type="disc">
<li class="gmail-m_-2898440583235044390gmail-m954207991152786889msolistparagraph">
I do not want upstream developers "trying to be polite" if that delays otherwise worthwhile work.<u></u><u></u></li></ul>
<p class="MsoNormal">Nobody suggested that. It’s perfectly possible to “be polite” and still not delay worthwhile work.<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<ul type="disc">
<li class="gmail-m_-2898440583235044390gmail-m954207991152786889msolistparagraph">
The current policy is "downstream is on their own".<u></u><u></u></li></ul>
<p class="MsoNormal">Nobody disputes that.<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<ul type="disc">
<li class="gmail-m_-2898440583235044390gmail-m954207991152786889msolistparagraph">
There was nothing even remotely unreasonable done in the patch series which triggered this discussion and I don't want any upstream contributor coming to believe there was.<u></u><u></u></li></ul>
<p class="MsoNormal">I disagree with you about “nothing even remotely unreasonable” in that I think it was done slightly unreasonably, and I think it’s worthwhile to have other upstream contributors
acknowledge that feeling.<u></u><u></u></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">What I’m aware of is a series of 5 separate patches, each of which re-capitalized a select subset of APIs in the same area. This fixing-up is *<b>long overdue</b>* and I’m very
happy to see it done. But I will dispute the “reasonableness” of doing it in 5 separate stages (plus a few other related cleanups that were not simply re-capitalizing existing names).<u></u><u></u></p>
<p class="MsoNormal">The notion of “reviewable size” is not relevant, because these patches were not reviewed (in accordance with current policy, that this kind of mechanical fixup is “obvious” and
does not require pre-commit review).<u></u><u></u></p>
<p class="MsoNormal">The patches were not really very distinct, with a HUGE overlap in the set of affected files.<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">We do like to minimize churn; this kind of fixup clearly qualifies as churn, but I respectfully submit that the way it was done does not *<b>minimize</b>* the churn.<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">And minimizing the churn is good for both the project and those of us who live downstream of it.<u></u><u></u></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">If you ignore downstream users, and focus on upstream: what makes it the least risky and most convenient for the *upstream contributor* in this case? It seems they believed it was to split their patch up, which I would have done as well:
if you can split it, then in most case you should.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">If the patches can land separately, to me their are distinct (they are not unrelated, but that's not a reason for merging them in a single commit). Just like David mentioned separately, I may have done this over a much longer series of
individual commits.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">If you are arguing that the reason to not split these in separate patch is about downstream users, then I am with Philip: you start to creep up some expectations that are going against the current status quo in which (in my opinion) downstream
shouldn't impact upstream development.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Best,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">-- <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Mehdi<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal">--paulr<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<div style="border-style:none none none solid;border-left-width:1.5pt;border-left-color:blue;padding:0in 0in 0in 4pt">
<div>
<div style="border-style:solid none none;border-top-width:1pt;border-top-color:rgb(225,225,225);padding:3pt 0in 0in">
<p class="MsoNormal"><b>From:</b> llvm-dev <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>>
<b>On Behalf Of </b>Philip Reames via llvm-dev<br>
<b>Sent:</b> Wednesday, February 19, 2020 2:07 PM<br>
<b>To:</b> Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>><br>
<b>Cc:</b> llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>; Valentin Churavy <<a href="mailto:v.churavy@gmail.com" target="_blank">v.churavy@gmail.com</a>><br>
<b>Subject:</b> Re: [llvm-dev] amount of camelCase refactoring causing some downstream overhead<u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<p>Eric,<u></u><u></u></p>
<p>I disagree. Strongly. I see the very fact we're engaging in the discussion of "just being polite" here as normalizing a proposed change in policy which has potentially profound negative consequences for the project long term. I do not want upstream developers
"trying to be polite" if that delays otherwise worthwhile work. The current policy is "downstream is on their own". There was nothing even remotely unreasonable done in the patch series which triggered this discussion and I don't want any upstream contributor
coming to believe there was.<u></u><u></u></p>
<p>Again, I'm open to carefully weighted proposals to change current policy. I also have a downstream repo which is kept up to date and I understand the pain point being raised. I just want to be very careful to distinguish between existing status, and any
proposed changes. I want the proposed changes to be carefully weighed before being put into effect.
<u></u><u></u></p>
<p>Philip<u></u><u></u></p>
<div>
<p class="MsoNormal">On 2/18/20 4:39 PM, Eric Christopher wrote:<u></u><u></u></p>
</div>
<blockquote style="margin-top:5pt;margin-bottom:5pt">
<div>
<p class="MsoNormal">Hi Philip,
<u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">While it's true we don't I think Valentin is reasonable in saying "hey, when people do this let's try to combine them if it makes sense". It's just being polite to everyone, especially
if it doesn't risk the patches or upstream stability. I don't think there's a policy change being proposed, just a "hey, let's see what we can do in the future".<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">-eric<u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">On Tue, Feb 18, 2020 at 4:05 PM Philip Reames via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<p>Valentin,<u></u><u></u></p>
<p>You are proposing to change existing policy. Current policy is that we don't consider downstream *at all*. Your proposal may seem reasonable - it may even *be* reasonable - but it is definitely a change from historical practice and must be considered as
such. <u></u><u></u></p>
<p>Philip<u></u><u></u></p>
<div>
<p class="MsoNormal">On 2/18/20 3:03 PM, Valentin Churavy wrote:<u></u><u></u></p>
</div>
<blockquote style="margin-top:5pt;margin-bottom:5pt">
<div>
<p class="MsoNormal">I don't think anyone is arguing to change longstanding policy. From a downstream perspective many small renaming changes do increase overhead for us.
<u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">One thing that happens to downstream projects is that they support more than one LLVM version, we (JuliaLang) currently try to support latest stable + master.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">So for me the question is more, are renaming changes worth downstream projects not being able to test and provide feedback to upstream? One way of mitigating that is to consciously
schedule them just before a release and do them all in short succession.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">-V<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">On Tue, Feb 18, 2020, 17:00 Philip Reames via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<p class="MsoNormal">As others have said, our long standing policy has been that downstream
<br>
projects must fend for themselves. We're certainly not going to reverse <br>
that policy without careful discussion of the tradeoffs.<br>
<br>
I'm personally of the opinion that there could be a middle ground which <br>
allows upstream to move quickly while reducing headache for downstream <br>
projects. Given I wear both hats, I know I'd certainly appreciate such <br>
a state. However, it's important to state that such decisions would <br>
need to be carefully considered and would require some very careful <br>
drafting of proposal to balance the competing interests at hand.<br>
<br>
If anyone is curious, I'm happy to share some ideas offline on what <br>
starting points might be, but I have neither the time nor the interest <br>
to drive such a conversion on list.<br>
<br>
Philip<br>
<br>
On 2/18/20 1:37 AM, Ties Stuij via llvm-dev wrote:<br>
> During that variable renaming debate, there was a discussion about discussion about doing things all at once, piecemeal or not at all. An issue that wasn't really resolved I think. I had the impression that the efforts fizzled out a bit, and I thought this
renaming was maybe related to that, but I'm neutral on if we should do variable renaming.<br>
><br>
> All I'm asking as a kindness if we could be kind on poor downstream maintainers not on the issue of variable renaming at large, but on the micro level of not pushing 5/6 patches of this kind covering closely related functionality in two days but collating
them in 1. I don't think that would slow down development, and I wanted to highlight the issue, as people might not be aware that they could save some pain in a simple way. Especially if indeed there is going to be a big renaming push and this would be a continuous
thing.<br>
><br>
> Cheers,<br>
> /Ties<br>
><br>
> ________________________________________<br>
> From: Michael Kruse <<a href="mailto:llvmdev@meinersbur.de" target="_blank">llvmdev@meinersbur.de</a>><br>
> Sent: 17 February 2020 21:16<br>
> To: Ties Stuij<br>
> Cc: llvm-dev<br>
> Subject: Re: [llvm-dev] amount of camelCase refactoring causing some downstream overhead<br>
><br>
> My understanding is that LLVM's general policy is to not let<br>
> downstream slow down upstream development. The C++ API explicitly<br>
> unstable.<br>
><br>
> Note that we are even considering renaming variables globally:<br>
> <a href="https://lists.llvm.org/pipermail/llvm-dev/2019-September/134921.html" target="_blank">
https://lists.llvm.org/pipermail/llvm-dev/2019-September/134921.html</a><br>
><br>
> Michael<br>
><br>
> Am Mo., 17. Feb. 2020 um 06:04 Uhr schrieb Ties Stuij via llvm-dev<br>
> <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>:<br>
>> Hi there,<br>
>><br>
>> At the end of last week we saw a number of commits go in that were camelCasing batches of MCStreamer::Emit* and AsmPrinter::Emit* functions.<br>
>><br>
>> For example:<br>
>> - <a href="https://reviews.llvm.org/rG549b436beb4129854e729a3e1398f03429149691" target="_blank">
https://reviews.llvm.org/rG549b436beb4129854e729a3e1398f03429149691</a><br>
>> - <a href="https://reviews.llvm.org/rGa55daa146166353236aa528546397226bee9363b" target="_blank">
https://reviews.llvm.org/rGa55daa146166353236aa528546397226bee9363b</a><br>
>> - <a href="https://reviews.llvm.org/rG0bc77a0f0d1606520c7ad0ea72c434661786a956" target="_blank">
https://reviews.llvm.org/rG0bc77a0f0d1606520c7ad0ea72c434661786a956</a><br>
>><br>
>> Unfortunately all these individual commits trigger the same merge conflicts over and over again with our downstream repo, which takes us some manual intervention every time.<br>
>><br>
>> I understand uniformity is a nice to have, but:<br>
>> 1 - is it worth it to do this work right now? I can remember the casing debate a few months back, which seems unrelated to this work which seems manual, but I'm unsure of the outcome.<br>
>> 2 - If this work should be done, it would be nice if all of the work is done in one batch, to save us some of the downstream overhead.<br>
>><br>
>> Thanks<br>
>> /Ties<br>
>> _______________________________________________<br>
>> LLVM Developers mailing list<br>
>> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><u></u><u></u></p>
</blockquote>
</div>
</blockquote>
</div>
<p class="MsoNormal">_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><u></u><u></u></p>
</blockquote>
</div>
</blockquote>
</div>
</div>
</div>
<p class="MsoNormal">_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><u></u><u></u></p>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote></div></div>