On 28/01/2020 23:15, Joel Sherrill wrote:
>
>
> On Tue, Jan 28, 2020, 5:22 AM Christian Mauderer
> <christian.maude...@embedded-brains.de
> <mailto:christian.maude...@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 <chr...@rtems.org
> <mailto:chr...@rtems.org>
> >>>> <mailto:chr...@rtems.org <mailto:chr...@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.
>
You most likely already noted: I don't like style discussions. That's
why I said that I'm indifferent which style we use. You are right that
any well documented and fixed style should do as long as some automatic
checks can be used.
>
> 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?
>
You are right. The correct statement would have been: "It is less likely
to find a tool that formats or checks the style we want."
So a style and a tool should be selected together.
For yapf it seems to be possible to have an influence on _some_
parameters using a config file.
> 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@rtems.org <mailto:devel@rtems.org>
> <mailto:devel@rtems.org <mailto:devel@rtems.org>>
> >>>> http://lists.rtems.org/mailman/listinfo/devel
> >>>>
> >>> _______________________________________________
> >>> devel mailing list
> >>> devel@rtems.org <mailto:devel@rtems.org>
> >>> http://lists.rtems.org/mailman/listinfo/devel
> >>>
> >>
>
> --
> --------------------------------------------
> embedded brains GmbH
> Herr Christian Mauderer
> Dornierstr. 4
> D-82178 Puchheim
> Germany
> email: christian.maude...@embedded-brains.de
> <mailto: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.
>
--
--------------------------------------------
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
