Requirement Document generator tool

[Christian Mauderer]

On 30/01/2020 23:13, Chris Johns wrote:
> On 30/1/20 8:35 pm, Christian Mauderer wrote:
>> On 29/01/2020 23:48, Chris Johns wrote:
>>> On 29/1/20 7:40 pm, Christian Mauderer wrote:
>>>> On 28/01/2020 23:15, Chris Johns wrote:
>>>>> On 28/1/20 10:22 pm, Christian Mauderer wrote:
>>>>>> I'm quite indifferent which style we use
>>>>>
>>>>> But you are arguing a position, such as ...
>>>>>
>>>>>> but do you really think that it
>>>>>> is a good idea to roll our own RTEMS-Python-style instead of using one
>>>>>> of the big ones (like PEP8, Google or some other that might is a better
>>>>>> fit for existing code). Rolling our own would lead to:
>>>>>>
>>>>>> 1. Long discussions about what is THE RIGHT STYLE (already started in
>>>>>> this mail).
>>>>>>
>>>>>> 2. A lot of problems that there is no tool that formats or checks
>>>>>> whether code is in THE RIGHT STYLE.
>>>>>>
>>>>>> 3. More discussions later because some other developer thinks that THE
>>>>>> RIGHT STYLE is wrong.
>>>>
>>>> You are right that I argue against home-grown styles. I learned that
>>>> style discussions are quite similar to religious discussions. Everyone
>>>> beliefs that his style is the only right one. Using a big one makes it
>>>> simpler to avoid discussions because a lot of people know it.
>>>
>>> And yet you inherit issues a large organisation cannot correct because
>>> historically a style is difficult to change and they are wedged. They need to be
>>> reviewed and evaluated.
>>
>> That's why I originally thought of defining a style for _new_ code with
>> an optional _slow_ migration if some existing code is touched. Of course
>> the style should be compatible with the views how the old code should
>> look in the future too. 
> 
> I do not think this works as I pointed out earlier. We leave a libaility in the
> old code that someone needs to clean up or we just ignore the rules for the old
> code. Either are not great.

I agree that it's not a great solution. But we have existing code. And
like you said yourself: It's not a good idea to just blindly reformat
it. And I assume even if it would be in the scope of some project, we
wouldn't entirely trust a new developer to do it right because we don't
know how careful he works.

> 
>> The patches were just for an estimate how much
>> influence the suggested styles could have. I would never expect that any
>> big patch with automated changes would be acceptable without a thorough
>> review. I wouldn't accept that myself.
> 
> I understand this.
> 
>>>> Please don't get me wrong: I'm OK with another style too. The relevant
>>>> result is that we have _one_ style in RTEMS that can be automatically
>>>> checked. 
>>>
>>> I find it interesting this view is OK for Python and the tools and yet we do not
>>> have automatic checking for the score's C. Or do we?
>>
>> It would be great if we would have automatic checking for C too. There
>> are big projects that have something like that and it's really
>> convenient. For example for Linux before you send a patch you can just
>> run "./scripts/checkpatch.pl some.patch" and it will give you everything
>> you should fix before sending it to the list. That removes at least one
>> round of review.
> 
> My point is taking say Google C or C++ coding standard and applying it to the
> score. I would prefer we wrote up the rules we have in place and check those.

Ah OK. I missed your point. In that case: I'm a bit torn. I wouldn't say
that it is always a bad idea to migrate to some adapted style rules if
there are good reasons to drop the old ones (except from a revision
control system view where such changes are always annoying). But please
also note that the differences would be a lot bigger.

I counted the lines in the files that I touched with the formatting tool:

wc -l `find -name "*.py" | grep -v "patch-gdb-python" | grep -v "tftpy"
| grep -v "asciidoc"`

It's a total of 18193 lines. The patch touched about 3000 of them. So
it's not a small part but it's not everything either. I assume that we
can reduce it as soon as we worked out some rules and solutions for
formats that can't be touched to the extend that these coding standards
would.

For RTEMS score and nearly every C coding style I know it would be more
of 90 to 100 percent of the code due to the indentation style alone.

> 
>> But I think implementing that would be a lot bigger task than the one we
>> are just discussing. One of the first problems would be finding a style
>> checker that can produce _exactly_ the coding style that is defined for
>> score.
> 
> Agreed.
> 
>> And also I'm happy to discuss approaches for something like that
>> it would start to be of-topic for this thread. So if you want to discuss
>> details about automatic style checking for score we should create an
>> extra topic. Maybe we could even create a GSoC project for something
>> like that?
> 
> That is a good idea. Would you like to raise a task ticket?

I can do that. I think it could be a nice project. If it works we gain
quite a value from it. If it doesn't work we don't lose anything.

> 
>>>> That also will make patch review simpler because we as
>>>> maintainers don't have to do the style review by hand.
>>>
>>> This is often more about those coding than those reviewing.
>>
>> Sorry, I think I don't fully understand you here.
> 
> The reviewers often know the rules and can code to them, it is those writing the
> code that do not and need the help. A formatter helps them.

OK. I agree here. That's about the same that I said for the linux
checkpatch.pl script. It's really convenient to avoid basic bugs.

> 
>>>>> I wrote the code, I know it and I need to maintain it. All I am asking is if
>>>>> pre-qual wants rules then I suggest they find some common ground.
>>>>
>>>> I agree that it is a good idea to find something that is acceptable for all.
>>>>
>>>>>
>>>>> I wonder how libbsd.py goes with these rules. Also all the wscript files which
>>>>> are also python and also the RSB would be covered.
>>>>
>>>> Most likely not well. But that's the problem: We already have a bunch of
>>>> different styles. Let's try to agree on one before we get lots of extra
>>>> code. A lot of pre-qualification code will be python. Therefore now is
>>>> the best time.
>>>
>>> You may know more than me about this. We have been given no information about
>>> the project's plans or these tools and is becoming frustrating. Joel and I have
>>> asked on a number occasions. The silence is testing our patience.
>>
>> I'm really sorry for that. Like I mentioned earlier I'm not really very
>> good with the overview over this project too. I'll try to discuss this
>> with Sebastian and Thomas as soon as Sebastian is back from vacation.
> 
> I understand you are not. I just want to make sure the whole project knows this
> is important.
> 
>>> How about we put a python rules discussion on hold until the intentions of the
>>> pre-qual project are presented publicly and in a manner we can all digest. I
>>> have spent enough time and energy discussing this stuff and it is not clear to
>>> me we are in a position to accept what is to be written.
>>
>> If you want to stop the discussion, I have to accept that. Please just
>> answer with "this needs further discussion but not now" for all parts
>> where you think we should stop right now.
> 
> I think you and I are close but neither of us are doing the work so I am not
> sure where that leaves us.

Thanks for your patience in this discussion.

You are right. We can agree to something but it wouldn't necessarily
mean that all others agree too. On the other hand it's an open
discussion and we don't stop anyone from participating. If someone else
suggests something different afterwards there are at least two of us
that can argue into the same direction ;-)

I assume that Sebastian will add some opinion too as soon as he is back
from vacation next week.

> 
>> But please note that our current discussion is in my opinion more of a
>> basic discussion that is only triggered by the pre-qualification
>> efforts. It's not really relevant whether new tools are added by
>> pre-qualification in the next year or for some other reasons in the next
>> five or ten years. Our Python code base is growing and it will become
>> harder to migrate to some common style with each added snipped of Python
>> code.
> 
> This is all valid and I am not questioning the benefit.

Good to hear. I started to feel bad for arguing so hard for something
that isn't wanted.

> I am concerned about the
> amount of work this creates in the project not covered by the pre-qual team. If
> this was a GSoC project I was mentoring I would make sure the student had access
> to a Windows, Mac, FreeBSD and Linux box and I would design a series of steps
> where each file is converted then tested and the results checked. It would be a
> slow ad time consuming task but if automated it may work.

I'm sure that it's no small task. But I think that it would be possible
to break it down into smaller ones. There are changes where it is enough
to take a quick look to tell: Yes that works. For example the additional
new lines between functions like suggested by both discussed standards.
I think these parts could be integrated without problems. But you are
right that the rest of the changes are a bit of a challenge.

> 
>> To come back to your argument regarding C code: For score there is at
>> least a defined coding style that could be used as a basis for automatic
>> checks. We currently don't have a defined or recommended style for
>> Python. So we will have a lot less uniform Python code base than we have
>> in score in a few years.
> 
> For the "our" code of which I wrote most of it the style has moved a bit but not
> much. It is not based on anything specific, it is more like C coding in python.
> I do not claim to be an expert and it is clearly home grown.

I know that a lot of code is from you and I didn't want to play that
down. Most likely we have a lot of your style because you wrote the
biggest parts. But I'm sure that there are quite some parts that don't
follow that style too.

Another part is: I know from myself that if there is no checker, I
always tend to bend the rules a bit. And depending on the year it's more
or less. So I wouldn't be able to extract fixed rules from my code that
I could give to a formatter and have an output with really few changes.

> 
>>>> If I understood you correctly your main problem with both suggested ones
>>>> (PEP8 and Google) is that multi line lists have been put together into
>>>> one liners and dictionaries with aligned fields are not aligned anymore?
>>>
>>> They cannot be touched ...
>>>
>>> https://git.rtems.org/rtems-source-builder/tree/source-builder/sb/darwin.py#n41
>>> https://git.rtems.org/rtems-source-builder/tree/source-builder/sb/freebsd.py#n50
>>> https://git.rtems.org/rtems-source-builder/tree/source-builder/sb/linux.py#n54
>>> https://git.rtems.org/rtems-source-builder/tree/source-builder/sb/linux.py#n104
>>> https://git.rtems.org/rtems-source-builder/tree/source-builder/sb/linux.py#n104
>>> https://git.rtems.org/rtems-source-builder/tree/source-builder/sb/options.py#n56
>>> https://git.rtems.org/rtems-libbsd/tree/libbsd.py#n59
>>> https://git.rtems.org/rtems-libbsd/tree/libbsd.py#n108
>>> https://git.rtems.org/rtems-libbsd/tree/libbsd.py#n148
>>>
>>> I could go on but I think this list should highlight the issue.
>>>
>>
>> I agree that the readability would suffer for cases like that. So we
>> will have to find a solution for that.
> 
> Yes, a tricky problem to solve. I only extracted this from your post to
> highlight the more difficult things to face.
> 
>>>> About correct?
>>>
>>> The lack of space around the arguments is also a change that is silly and I do
>>> not want changed.
>>
>> I think that is based on the "official" python style (which is PEP-0008):
>>
>> https://www.python.org/dev/peps/pep-0008/#whitespace-in-expressions-and-statements
> 
> Oh interesting, thanks. I had not made the connection that the pep8 format was
> from here.

I thought that it is well known that the PEPs are some kind of python
standard. Shouldn't have assumed that.

> 
> The combining arg annotations with the default value makes me sigh, a rule
> within a rule.
> 
> I could not see dict layouts in it.

You are right. I didn't find them either. So maybe we are lucky that it
depends on some formater options rather than style.

Sebastian also mentioned that some formaters (for example "black") can
have special comments to tell the formater to ignore a block of code (in
case of "black" with "# fmt: on/off").

"yapf" has a similar one that can even be used for small parts like
array initializations:
https://pypi.org/project/yapf/#why-does-yapf-destroy-my-awesome-formatting

In the chapter "A Foolish Consistency is the Hobgoblin of Little Minds"
PEP8 specially mentions that the style shouldn't be applied if it would
be less readable. So switching the formater off for small parts would
even be according to the style guide. Google don't expresses it that
explicitly but the "Parting Words" suggest something similar.

Please also note that PEP8 explicitely tells "Some other good reasons to
ignore a particular guideline: [...] 3. Because the code in question
predates the introduction of the guideline and there is no other reason
to be modifying that code." So that style even suggests to not touch
existing code if it isn't touched anyway due to another reason.

> 
>>> The biggest challenge who ever finally proposes these rules faces is the RSB. It
>>> is very difficult to know if you have broken it. It has taken years to converge
>>> to the level of stability we have. All changes in here would need a close review
>>> and that something I am not excited about.
>>
>> I agree that all changes should be reviewed closely. Again: I'm in favor
>> of finding something acceptable that can be used for new tools and then
>> try to start a slow migration maybe even over years. Note that this is a
>> purely personal preference and I don't have an idea whether it matches
>> the targets of pre-qualification or not.
> 
> Are you volunteering to spend years making the change? So far it is just you and
> me. ;)
> 
> I see for the existing python a series of options:
> 
> 1. Do not allow rules or changes to the formatting until all existing code has
> been migrated, tested and accepted

I'm not in favor of that. We should agree to a set of rules _before_
changing any code. Otherwise: How would we know the direction for the
changes.

> 
> 2. Allow a set of rules and all new code follows the rules and we ignore the
> existing code
> 
> 3. Slowly migrate the existing code over testing each file and merging when tested

I think my personal preference would be a combination of 2 and 3: Find a
style that is acceptable for all of us and use it for new code. If
bigger blocks of old code are touched: Migrate it to the new style.

> 
> 4. Covert all existing code and let everyone handle the fall out if there is any

Maybe applying _some_ of the uncritical rules with only easily
reviewable changes could be a good step to reduce the difference and
improve the readability. But I agree that it mustn't be changes that
compromise stability.

> 
> 5. Do nothing

Not a good solution if the target is to get anything done.

> 
> 6. Something else?
> 
> None of these are ideal as someone is effected and some are not realistic.
> 
> Chris
> 

Best regards

Christian
-- 
--------------------------------------------
embedded brains GmbH
Herr Christian Mauderer
Dornierstr. 4
D-82178 Puchheim
Germany
email: christian.mauderer at embedded-brains.de
Phone: +49-89-18 94 741 - 18
Fax:   +49-89-18 94 741 - 08
PGP: Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.