Re: Patch contribution tips


Carles Cufi
 

Hey Raúl,

 

The issue with your PR has nothing to do with twister.

 

The issue is that you have lines in your commit message that exceed the maximum length permitted.

 

In order to fix that, just do:

 

$ git commit –amend

<split the lines>

$ git push -f origin

 

8: UC4 Line exceeds max length (182>72): "nucleo_g0b1re.dts.pre.tmp:1749.36-1752.5: Warning (unit_address_format): /soc/flash-controller@40022000/flash@8000000/partitions/partition@0C000: unit name should not have leading 0s"

259: UC4 Line exceeds max length (146>72): "warning: unit address and first address in 'reg' (0x3e000) don't match for /soc/flash-controller@40022000/flash@8000000/partitions/partition@31000"

 

 

From: devel@... <devel@...> On Behalf Of Raúl Sánchez Siles via lists.zephyrproject.org
Sent: 24 March 2021 17:03
To: devel@...
Cc: devel@...
Subject: Re: [Zephyr-devel] Patch contribution tips

 

  Hello all:

 

  Thanks for your pointers and comments. They are really appreciated.

 

  Regarding the twister tool I was aware of its existence. Maybe a more precise question is whether you have a suggested parameter set or configuration that would mimic what a potential automatic testing does. If someone wanting to contribute is able to run twister this way, it would be easier to make sure that the proposed changes complies with the expected quality level.

 

  Maybe you just do:

 

 ./scripts/twister --all --enable-slow

 

  As stated in the documentation. I have tried this before but I have plenty of errors and that is why I guessed you may be doing it differently.

 

  Regarding the compliance checks I do understand and share the need of looking at automated checks results. Maybe my proposal would be adding support for some kind of verbatim markup tag. Maybe this is just bloated and there so little cases of justified warnings that it does not deserve the effort.

 

  Thanks for your work keeping the project going, especially to those maintainers and release engineers.

 

  Best regards,

 

El miércoles, 24 de marzo de 2021 9:54:34 (CET) Erwan Gouriou via lists.zephyrproject.org escribió:

 

Hi Raùl,

 

Regarding the question about a way to run full non regression test, as mentioned by Jennifer, advice is to 

use dedicated tool called twister:

https://docs.zephyrproject.org/latest/guides/test/twister.html

 

About the warnings reported by compliance checks, we can indeed argue these are of minor importance 

and could be derogated (especially in this case where effort was made to clearly document the change).

One reason not to derogate and do the requested change is that maintainers and release engineers (the 

ones who merge) bandwidth is limited,  so we're heavily relying on automated checks and verdicts. 

This allows using github filters to get a list of PRs that are fulfilling all merge criterias. For instance,

here is the filter reporting PRs ready for merge:

https://github.com/zephyrproject-rtos/zephyr/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-desc+review%3Aapproved+-label%3ADNM+status%3Asuccess++draft%3Afalse+base%3Amaster

(At the time I'm writing these lines, 17 PRs waiting for merge > list is long).

 

So even if derogating rules with automated checks would make sense in some cases, this actually requires

time to deviate from the process (ping people, agree that in that particular case deviation is justified, ....).

And of course, the more derogations are done, the more derogations are requested.

So getting compliance checks happy saves time (and then that time could be better spent in other tasks).

 

Hope it helps.

Erwan

 

On Tue, 23 Mar 2021 at 21:21, Jennifer M Williams <jennifer.m.williams@...> wrote:

 

Hi Raúl,

 

Thanks for your contribution! Welcome to the community. Please take a look at this information on the project docs https://docs.zephyrproject.org/latest/contribute/index.html. Sounds like you would be interested in using Twister to run many of the tests the CI system runs.

 

I encourage others to reply as well, but I had a minute to drop in with something hoping to get you going.

 

Cheers!

Jen

 

From: devel@... <devel@...> On Behalf Of Raúl Sánchez Siles

 Sent: Tuesday, March 23, 2021 12:06 PM

 To: devel@...
Subject: [Zephyr-devel] Patch contribution tips

 

  Hello:

 

  I have dared to create my first contribution [1]. Unfortunately I think there are some missing bits I would need to figure out somewhere. I took a look at the documentation and did my best, but still some gaps are left to me to fill in.

 

[1] https://github.com/zephyrproject-rtos/zephyr/pull/33642 

 

  For this particular PR, the patch compliance check fails because I copied verbatim the error I tried to fix and some of the commit lines are longer than what is recommended and hence the check fails due to warnings. My first question is whether this kind of warnings are tolerated (for the PR to succeed) and if is there a way to hint the compliance checker to overlook such a case.

 

  For the above PR, I think the changes are simple enough but in case I happen to go with more intrusive changes I would like to know if there is a way to run the complete set of zephyr tests locally somehow. In other words the whole test set contained under the tests/ directory in the greatest possible extension, including all possible archs, boards, etc, ... (if this is possible at all). Perhaps you already do something like this automatically as part of the global CI pipeline.

 

  Lastly, if you have some more relevant tips about contribution I would like to read them.

 

  Best regards,

 

--

Raúl Sánchez Siles

 

SW Engineer

 

K-LAGAN EID

 

--

Raúl Sánchez Siles

 

SW Engineer

 

K-LAGAN EID

Join devel@lists.zephyrproject.org to automatically receive all group messages.