Requirement Document generator tool
Christian Mauderer
christian.mauderer at embedded-brains.de
Tue Jan 28 11:22:43 UTC 2020
On 28/01/2020 11:41, Chris Johns wrote:
>
>
> On 28/1/20 8:14 pm, Christian Mauderer wrote:
>> On 28/01/2020 01:42, Chris Johns wrote:
>>> On 28/1/20 10:48 am, Joel Sherrill wrote:
>>>> On Mon, Jan 27, 2020 at 4:18 PM Chris Johns <chrisj at rtems.org
>>>> <mailto:chrisj at rtems.org>> wrote:
>>>>
>>>> On 24/1/20 9:57 pm, Jose Valdez wrote:
>>>> > In fact these tools target the pre-qualified project.
>>>>
>>>> Do you see this as different to the RTEMS project?
>>>>
>>>> > Since it was Sebastian who suggested to create this set of python tools,
>>>>
>>>> I think Sebastian is wanting a smooth path for these tools into the project.
>>>>
>>>> > I think the idea was to standardize the use of python not only for this
>>>> project, but also for other python written code in RTEMS community. This has
>>>> the advantages that every written python code is standard, but has the
>>>> drawbacks:
>>>> >
>>>> > -> old written code would need to be adapted to the standards.
>>>>
>>>> How different to the proposed coding standard is the existing code? Why not base
>>>> the coding standard on what exists in the code base?
>>>>
>>>> This is a very important question.
>>>>
>>>> Have you evaluated the size of the task to update the existing code? How would
>>>> get such changes for the rtems-tools and the RSB be tested and integrated back
>>>> into the project? This apporach seems like a huge review task for me.
>>>>
>>>> It could be or it may turn out that there isn't much changed. Without someone
>>>> running the reformatter and reporting, we won't know.
>>>
>>> Running a reformatter may give you an idea of the scale however I have some
>>> concerns as Python's logic is based on indent levels and a missed level changes
>>> the logic. I am not sure how you know such a tool is safe to use unless you
>>> review all the changes.
>>
>> To get some statistics for that I tried using yapf on the rtems-tools
>> repo. I excluded everything with "patch-gdb-python", "tftpy" and
>> "asciidoc" in the name. You can see the yapf call in the commit message.
>
> The doxygen is from waf. Thomas knows how to write python.
OK. I missed that one. The patches were just a test to get some
orientation about what it would mean.
Please also note: I don't want to doubt the python knowledge of anyone.
>
>> Google style:
>> https://gist.githubusercontent.com/c-mauderer/18746eaa8f3a077adddfe35c1f7b5194/raw/f8202a53ca043d23732d79ab217071e975dc35ad/0001-FIXME-Format-in-Google-style.patch
>>
>> PEP8 Style:
>> https://gist.githubusercontent.com/c-mauderer/18746eaa8f3a077adddfe35c1f7b5194/raw/c97c77762f65041d9244bc8bc64b90ec698735bf/0001-FIXME-Reformat-using-PEP8.patch
>>
>> In both cases it's about 3000 lines removed and 3700 lines added.
>>
>> Without having a look at each line: Most seems to be just the normal
>> formatting stuff. Some lines are too long. Some spaces missing around
>> operators or spaces where they shouldn't be.
>
> And then there is removal of spaces on default args ...
>
> - def section_macro_map(self, section, nesting_level = 0):
> + def section_macro_map(self, section, nesting_level=0):
>
> I prefer spaces as it is consistent with other uses of spaces around operators.
In my view these are detail style discussions. Same is true for most of
your notes further below (although I agree with some of your opinions).
But I didn't wanted to trigger any of these. My intention was to give
some impression about how big such changes would be.
I know that there is quite some python code already in RTEMS. And I'm
also sure that there are already quite some different python styles in
RTEMS. At least as many as programmers touching python code. But
currently the python code is still few enough that it would be possible
to get a uniform style. And pre-qualification or not: I'm really
convinced that a fixed style would be a good thing for the code base in
general.
I'm quite indifferent which style we use 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.
>
>> Some lists written with
>> more or less line breaks.
>
> Yes to make them readable. This is on purpose. I do not agree with this ..
>
> - return cfgs + ['objcopy',
> - 'arch',
> - 'vendor',
> - 'board',
> - 'config_name',
> - 'first_stage',
> - 'second_stage',
> - 'kernel_converter']
> + return cfgs + [
> + 'objcopy', 'arch', 'vendor', 'board', 'config_name', 'first_stage',
> + 'second_stage', 'kernel_converter'
> + ]
>
>
>> Some extra line breaks added between functions.
>
> Yes, this seems to be some that happens. No spaces when setting an argument and
> then these spaces between functions and methods.
>
>> But I also noted a problem for python 3 compatibility: There are some
>> tab to space conversions. As far as I know python 3 throws at least a
>> warning or maybe an error if it finds tabs.
>
> Yes this happens and there are some cases in the code still.
>
> I do not like this ...
>
> - argsp.add_argument('--net-boot-file',
> - help = 'network boot file (default: %(default)r).',
> - type = str, default = 'rtems.img')
> - argsp.add_argument('--net-boot-fdt',
> - help = 'network boot load a fdt file (default:
> %(default)r).',
> - type = str, default = None)
> + argsp.add_argument(
> + '--net-boot-file',
> + help='network boot file (default: %(default)r).',
> + type=str,
> + default='rtems.img')
> + argsp.add_argument(
> + '--net-boot-fdt',
> + help='network boot load a fdt file (default: %(default)r).',
> + type=str,
> + default=None)
>
> This is a blocker for me ...
>
> - '_ncpus': ('none', 'none', ncpus),
> - '_os': ('none', 'none', 'darwin'),
> - '_host': ('triplet', 'required', uname[4] + '-apple-darwin' +
> uname[2]),
> - '_host_vendor': ('none', 'none', 'apple'),
> - '_host_os': ('none', 'none', 'darwin'),
> - '_host_cpu': ('none', 'none', uname[4]),
> - '_host_alias': ('none', 'none', '%{nil}'),
> - '_host_arch': ('none', 'none', uname[4]),
> - '_host_prefix': ('dir', 'optional', '%{_usr}'),
> - '_usr': ('dir', 'optional', '/usr/local'),
> - '_var': ('dir', 'optional', '/usr/local/var'),
> - '__ldconfig': ('exe', 'none', ''),
> - '__xz': ('exe', 'required', '%{_usr}/bin/xz'),
> - 'with_zlib': ('none', 'none', '--with-zlib=no'),
> - '_forced_static': ('none', 'none', '')
> - }
> + '_ncpus': ('none', 'none', ncpus),
> + '_os': ('none', 'none', 'darwin'),
> + '_host': ('triplet', 'required',
> + uname[4] + '-apple-darwin' + uname[2]),
> + '_host_vendor': ('none', 'none', 'apple'),
> + '_host_os': ('none', 'none', 'darwin'),
> + '_host_cpu': ('none', 'none', uname[4]),
> + '_host_alias': ('none', 'none', '%{nil}'),
> + '_host_arch': ('none', 'none', uname[4]),
> + '_host_prefix': ('dir', 'optional', '%{_usr}'),
> + '_usr': ('dir', 'optional', '/usr/local'),
> + '_var': ('dir', 'optional', '/usr/local/var'),
> + '__ldconfig': ('exe', 'none', ''),
> + '__xz': ('exe', 'required', '%{_usr}/bin/xz'),
> + 'with_zlib': ('none', 'none', '--with-zlib=no'),
> + '_forced_static': ('none', 'none', '')
> + }
>
> These cannot be touched. It would make the dicts really hard to maintain.
>
> Chris
>
>>
>>>
>>>> I tend to think it is worth knowing if this is a monster or a mouse before making
>>>> a decision.
>>>
>>> Yes this is important and also if it is a monster which specific rules make it
>>> so? It maybe most of the rules are fine.
>>>
>>> If these rules are important to the qual effort I hope they see the value in
>>> find these answers for us.
>>>
>>>> > Another option could be leave it as it is and only do this for new written
>>>> code.
>>>>
>>>> It would be confusing to any new user to the code to have code written to a
>>>> standard and code that is not? If you edit the old code is it to the new
>>>> standard? If you edit an old file do you need to update the whole file?
>>>>
>>>> If we accept a standard, then it is all or nothing. I'm going to sound like a
>>>> cranky old man but we have said things like this before and regretted it
>>>> every time. Consistency is critical.
>>>
>>> If you are a cranky old man then that must make me one as well. Ah the youth of
>>> today ... :)
>>>
>>>> Quick run of sloccount for a baseline
>>>>
>>>> + rtems-tools -
>>>> Totals grouped by language (dominant language first):
>>>> ansic: 47237 (49.86%)
>>>> cpp: 25837 (27.27%)
>>>> python: 21227 (22.40%)
>>>> sh: 442 (0.47%)
>>>
>>> Nice start. A more accurate report on this code would mean removing the imported
>>> pieces.
>>>
>>>> + rtems-source-builder/source-builder -
>>>> SLOC Directory SLOC-by-Language (Sorted)
>>>> 14314 sb python=14169,sh=145
>>>> 65 top_dir sh=65
>>>> 0 config (none)
>>>
>>> There are the .cfg and .bset files.
>>>
>>>> 0 patches (none)
>>>>
>>>> So we have about 35K SLOC or Python by that.
>>>>
>>>> No idea how the new standard versus the old looks. I thought Python had a consistent
>>>> style but I could be very wrong. :(
>>>
>>> I am not sure, I have never looked. My python skills have developed producing
>>> these tools and this code. I know doc comments are missing.
>>>
>>>> > -> at some point some tools need to be upgraded (ex: python 3.7 will
>>>> become unusable in 2030 Operating systems).
>>>>
>>>> I am not sure how this relates. Yes it will need to update however we need to
>>>> support python2 for user facing tools for a while yet. A lot of what we do and
>>>> how we work is historically driven.
>>>>
>>>> CentOS 8 was just released in October. None of the big organization users I
>>>> see are using it yet.
>>>>
>>>> We need to make a LTS release with 5 on Python 2 as a minimum. I feel strongly
>>>> about that.
>>>
>>> And I suspect longer.
>>>
>>>> As long as the tools are written in a python agnostic manner, the version won't
>>>> matter.
>>>
>>> Yes I agree, we should aim for a middle ground and avoid hitting any of issues
>>> that can arise.
>>>
>>>> We need some test cases for the tools to verify them
>>>
>>> Yes.
>>>
>>> Chris
>>>
>>>> > I hope soon to formalize our suggestion to you and then you may review it
>>>> (and propose changes if you find appropriate).
>>>>
>>>> I suggest working in the open and with us will be more beneficial in the
>>>> long term.
>>>>
>>>>
>>>> +1 I can't agree strongly enough.
>>>>
>>>>
>>>>
>>>> Note, I am assuming the remainder of the email was Christian's. The quoting from
>>>> your email client made it difficult to tell.
>>>>
>>>> Chris
>>>> _______________________________________________
>>>> devel mailing list
>>>> devel at rtems.org <mailto:devel at rtems.org>
>>>> http://lists.rtems.org/mailman/listinfo/devel
>>>>
>>> _______________________________________________
>>> devel mailing list
>>> devel at rtems.org
>>> http://lists.rtems.org/mailman/listinfo/devel
>>>
>>
--
--------------------------------------------
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.
More information about the devel
mailing list