Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't touch the style of generated files #6820

Merged

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Dec 19, 2022

Ideally the result of the generator would conform to the code style, but this would be difficult, especially with respect to the placement of line breaks in long logical lines. So, to avoid surprises when checking the style of generated files (which happens in releases and in long-time support branches), systematically skip generated files.

code_style.py reformats the templates, so the generated files are mostly ok. Preferably the parts that are constructed by Perl or Python code should respect the new code style (I've started on that and will make a PR later). However, generating code that conforms to the line length limit would be very difficult, and I don't propose to do that.

In development, this will prevent check-generated-files.sh from failing when we make a release and commit the generated files into Git, then run the CI. In 2.28, this will prevent check-generated-files.sh failing on the CI once the branch is built properly (regenerating files after the code style change).

There are other possible approaches. I think it would be realistic to insist on the code style of the generated files apart from the line lengths. But we'd need to test that adequately, because in the development branch the problem would only manifest at release time when we check the generated files into Git.

Gatekeeper checklist

Ideally the result of the generator would conform to the code style, but
this would be difficult, especially with respect to the placement of line
breaks in long logical lines. So, to avoid surprises when checking the style
of generated files (which happens in releases and in long-time support
branches), systematically skip generated files.

Signed-off-by: Gilles Peskine <[email protected]>
@gilles-peskine-arm gilles-peskine-arm added bug needs-review Every commit must be reviewed by at least two team members, needs-backports Backports are missing or are pending review and approval. needs-ci Needs to pass CI tests needs-reviewer This PR needs someone to pick it up for review size-s Estimated task size: small (~2d) component-test Test framework and CI scripts priority-high High priority - will be reviewed soon labels Dec 19, 2022
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mpg mpg removed the needs-ci Needs to pass CI tests label Dec 19, 2022
Copy link
Contributor

@wernerlewis wernerlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wernerlewis wernerlewis added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Dec 19, 2022
@mpg mpg removed the needs-backports Backports are missing or are pending review and approval. label Dec 19, 2022
@mpg mpg merged commit 82dad10 into Mbed-TLS:development Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports bug component-test Test framework and CI scripts priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants