On Fri, Jan 31, 2020 at 12:39 AM Christian Mauderer <christian.maude...@embedded-brains.de> wrote: > > 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. > +1 as a potential GSoC project. Please ping me with the ticket # so I can review it.
It could also explore different ways to accommodate Python style variations, perhaps to help define a "minimally divergent" rule set that intersects current rtems python use with some well-known styles. > > > >>>> 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.maude...@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. > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel