kicad-developers team mailing list archive

[Cirilo Bernardo <cirilo.bernardo@xxxxxxxxx>]

On Thu, Apr 13, 2017 at 1:34 PM, Clemens Koller <cko@xxxxxxxxx> wrote:
> Hi!
>
> These lines scream for some comments in the source...
> I wouldn't get it, too.
>
> Regards,
>
> Clemens
>

What sort of comment: "this is really supposed to have two sequential
calls to the same function, so don't change it"?  For me that makes no
sense. If anyone is going to play with parsers they should be familiar
with the standard that is being implemented; making changes without
understanding the specification (and without understanding what the
parser is doing) cannot possibly be a good thing. Programmers should
also check that they are fixing a demonstrable problem and if they don't
understand the code then it should be left alone. At any rate there is a
comment only a few lines back about how the unimplemented features
PROTO and EXTERNPROTO are to be treated and from this the fact
that PROTO and EXTERNPROTO code are different and in two distinct
blocks should suggest that they really should not be the same code.
In the past when other developers have seen code that they don't
immediately understand but which looks a little strange to them, they
at least ask for comments rather than making blind changes. If people
don't read an obvious comment block a few lines up I don't see why they
would read comments immediately surrounding code either. How much
hand-holding are we expected to do?

- Cirilo


> On 2017-04-13 14:03, Wayne Stambaugh wrote:
>> Cirilo,
>>
>> Thanks for the info.  The second call to ReadName() does look a bit odd
>> so I can understand Konrad's confusion.
>>
>> Cheers,
>>
>> Wayne
>>
>> On 4/12/2017 6:12 PM, Cirilo Bernardo wrote:
>>> Do not accept this patch, it will break the parser. The statement
>>> which was removed is not redundant.
>>>
>>> - Cirilo
>>>
>>> On Wed, Apr 12, 2017 at 8:01 PM, Konrad Beckmann
>>> <konrad.beckmann@xxxxxxxxx> wrote:
>>>> ---
>>>>  plugins/3d/vrml/v2/vrml2_base.cpp | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>>
>>>> _______________________________________________
>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>> More help   : https://help.launchpad.net/ListHelp
>>>>
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~kicad-developers
>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> More help   : https://help.launchpad.net/ListHelp
>>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp