[squid-dev] Fwd: [PATCH] for loops modernization

[khaled belhout]

hello Alex
I fixed the naming of the current iteration object, limited the
changes to "src/adaptation" sub tree and replaced NULL macro by
nullptr keyword.



2017-03-13 14:45 GMT+01:00 Alex Rousskov <rousskov at measurement-factory.com>:
> On 03/12/2017 08:42 PM, khaled belhout wrote:
>> I used clang-tidy tool to modernize all these loops.
>> http://clang.llvm.org/extra/clang-tidy/
>
> It looks like that tool is not ready for fully automated use. If you
> want to fix its results, please use "const auto" where possible and
> avoid using "i" for naming the current iteration object.
>
>
>> we can take the advantage of the tool by selecting the changes that
>> make code more readable understandable and maintainable.
>
> Please do not misinterpret my earlier comments as an argument against
> range loops. Range loops are good and new code should use them!
>
> However, I doubt the advantages of changing those old loops outweigh
> cross-branch development costs right now. Others may disagree, and, if
> they do, I would not object to a polished patch being committed.
>
>
> Thank you,
>
> Alex.
>
>
>> 2017-03-12 16:31 GMT+01:00 Alex Rousskov:
>>> On 03/12/2017 07:45 AM, khaled belhout wrote:
>>>
>>>> this patch modernize for loops using c++11 Range-based for loop
>>>
>>> Please use "const auto" where possible and avoid using "i" for naming
>>> the current iteration object.
>>>
>>> I am curious why did you decide to change all these loops? How did you
>>> select the loops to change? Normally, we avoid wide-spread polishing
>>> touches to minimize the price developers working with older code have to
>>> pay when porting back various bug fixes and features. I am trying to
>>> decide whether the advantages of changing these loops outweigh those
>>> costs in this case.
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: adaptation_range_for_and_nulptr_auto.patch
Type: text/x-patch
Size: 79201 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20170315/1bb0465a/attachment-0001.bin>