Requirement Document generator tool
Joel Sherrill
joel at rtems.org
Tue Jan 28 22:15:04 UTC 2020
On Tue, Jan 28, 2020, 5:22 AM Christian Mauderer <
christian.mauderer at embedded-brains.de> wrote:
> 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.
>
Is it possible to make changes we think are ok and in both styles? That
would start to whittle down the style issues
>>
> >> 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:
>
In spite of your points below, taking one of those with a few exceptions
isn't a big deal. We have lots of people who have written C code for RTEMS
and only a few who have written any Python.
One thing from the pre-qualification effort that you have forgotten is that
we are supposed to document our decisions and rationale. If we say we use
Python style X with the following changes and that is supported by style
checking and formatting tools with the following arguments, then that's it.
If someone asks later, we point to the documentation.
That's what a more formal process is all about. Being able to justify
decisions later and bring new people on board. A new person can ask why but
we don't have to change.
> 1. Long discussions about what is THE RIGHT STYLE (already started in
> this mail).
>
Just looking at the examples, it looks like some long lines get split and
in other cases, it combined neatly formatted lines to make something too
long and unmanageable. What rules are acting here?
> 2. A lot of problems that there is no tool that formats or checks
> whether code is in THE RIGHT STYLE.
>
This is an unproven assertion. Do the tools take arguments to tune points?
I think we need to see which base style we agree with more. Bulk says the
two styles start with same handicap. But if style X has one rule we hate
that style Y does not, then maybe it is easy.
If both styles agree and we don't have a style checker timing option, then
MAYBE we suck it up.
> 3. More discussions later because some other developer thinks that THE
> RIGHT STYLE is wrong.
>
This is answered by documenting the decision, knowing the tooling, and
shutting down the discussion by pointing them to our rules and rationale.
Anyone can volunteer but that doesn't give the privilege of causing churn.
>
> >
> >> 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'
> > + ]
>
+1
>
> >> 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.
>
What rule breaks the Darwin line? At least that's what I think I see.
>
> > 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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20200128/c6129c03/attachment-0001.html>
More information about the devel
mailing list