Create a developer guide #126
Conversation
Describe the general structure of the CI and the CI scripts. Write a few points of advice. There could be plenty more. Write a bit of guidance on validating CI changes. Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
added
needs: review
needs: reviewer
priority-medium
size-xs
So much better than anything I found with Google! Signed-off-by: Gilles Peskine <[email protected]>
There was a problem hiding this comment.
Looks mostly good to me. Well written and clear and I learned a lot reading this.
Few small nits to sort out before approval. I am sorry I didn't use the suggestions feature for typos but for some reason this button does not appear on comment boxes for me in the "review view" but only when I look at the "files changed" tab, which I don't really understand but there we are.
developer_guide.md
Outdated
| What goes for `mbedtls` also goes for other repositories tested by `mbedtls-test`, such as the upcoming PSA crypto implementation repository. | ||
|
|
There was a problem hiding this comment.
Now that TF-PSA-Crypto has been released we should probably change the wording here and perhaps link to it as well.
Perhaps: "... such as the TF-PSA-Crypto repository."
developer_guide.md
Outdated
|
|
||
| #### Global variables | ||
|
|
||
| Note that Groovy does not have global variables as such. Each module (`*.groovy` file) is a class, and that class can have multiple instances. Therefore, avoid using script-scope variables in a Groovy module that is loaded from another module. There's existing code that does this, but it's fragile and has caused us headache so we are moving away from that. |
There was a problem hiding this comment.
'headache' wants to be plural here.
developer_guide.md
Outdated
|
|
||
| ## Validating changes | ||
|
|
||
| There is to continuous integration on the `mbedtls-test` repository (except a DCO check for the rare external contributions). Therefore, whenever you change the code, you must run some test jobs manually. What to run depends on what you're changing. |
There was a problem hiding this comment.
Typo: There is no continuous integration on the mbedtls-test repository...
developer_guide.md
Outdated
|
|
||
| If you make changes that affect error reporting, make sure that failures are still caught properly. We don't want to accidentally make a change that is fine if the tests pass, but hide failures! | ||
|
|
||
| There are pull requests for testing various kinds of failure in the [`mbedtls-restricted` repository](https://github.com/Mbed-TLS/mbedtls-restricted/labels/ci-testing) (private link). See [“CI testing: development, good”](https://github.com/Mbed-TLS/mbedtls-restricted/pull/906) for more information. |
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
I don't mind, it's more work to use that for typos anyway.
The button only appears when GitHub knows how to generate a usable patch. It appears when you look at the whole diff, but not always when you're looking at a subset of the commits. In particular it never appears if there's a commit that's not included in your view that affects the same lines, since the patch couldn't be applied to the pull request. |
Ah cool, good to know. Thanks. |
Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
dismissed stale reviews from tgonzalezorlandoarm and tom-daubney-arm
via
596b1eb
Signed-off-by: Gilles Peskine <[email protected]>
Thanks to recent improvements, it's now easier to test ABI-check and email reporting. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
There are no completeness goals here. I will take requests but I don't intend to spend much more time on this. Follow-ups are very welcome.