Requirement Document generator tool
Chris Johns
chrisj at rtems.org
Tue Jan 28 10:41:05 UTC 2020
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.
> 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.
> 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
>>
>
More information about the devel
mailing list