Code review comment for ~rafaeldtinoco/ubuntu/+source/iproute2:lp1831775-cosmic-sru-iproute2

Hello Andreas,

I pushed the commit again fixing changelog and version for cosmic which I saw it was wrong. Now, after reading your comments..

"""
Thanks, some comments inline, and below.

For SRUs, the merge target should always be ubuntu/<release>-devel.
"""

Okay

"""
Can you elaborate a bit more on the test case you added to the bug? Like, what to expect in the broken and in the good cases. Ideally it should be as simple as just copying and pasting something in the command line, but it's fine if it needs some setting up. For example, "create a connection from A to B, note the IPs used, and use the IPs in the following command."
"""

Alright, will change bug description for a more clear way of reproducing the issue

"""
I saw you added some test results further down in the bug, but for the SRU template, it's best if all that's needed to verify the bug is fixed is in the "opening salvo".
"""

I see.

"""
In "regression potential", you say " * ss interpreter (bison powered) could be broken". What does that mean? Why do you think it could be broken? These are questions the SRU team would probably ask. "ip" is used all over the place, and it's good to detail what are your thoughts on possible regressions.
"""

I usually start with biggest risk and try to reduce the risk with other topics. So, in this case:

 * ss interpreter (bison powered) could be broken

Means that the biggest risk would be to have ss interpreter broken.

 * patch is based in an upstream fix and was tested

But the patch, that changes ss interpreter, is an upstream fix and has been tested.

 * would likely not jeopardize ip command (higher issues)

It would NOT affect "ip" command and its interpreter, the most important part of iproute2.

But , now that I re-wrote in other words, I see the need to change that and will revisit.