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

chore(ruff): apply formatting and integrate into CI #5138

Merged
merged 25 commits into from
Feb 26, 2025

Conversation

johubertj
Copy link
Contributor

@johubertj johubertj commented Feb 20, 2025

Resolved issues:

resolves #5027

Description of changes:

  • Reformatted all tests/integrationv2 files using ruff
  • Replaced autopep8 with ruff as the default formatter in CI (ci_linting.yml).

Manual changes:

  • Replaced autopep8 with ruff in ci_linting.yml.

Automated changes:

  • Used uv run ruff format to format all files in tests/integrationv2.

Why replace autopep8 with ruff?

  • Ruff provides a rustfmt-like experience, meaning it enforces a standardized, opinionated code style with minimal configuration. Unlike autopep8, which primarily focuses on fixing formatting issues, ruff operates with a "set it and forget it" approach—developers do not need to fine-tune settings or make repeated manual fixes to satisfy linting rules.
  • Unlike autopep8, which only handles formatting, ruff also functions as a linter and static analysis tool, reducing the need for multiple dependencies.
  • Ruff is significantly faster than autopep8, improving efficiency in both local development and CI runs.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Feb 20, 2025
@johubertj johubertj changed the title Formatting changes Ruff Formatting and add to CI Feb 20, 2025
@johubertj johubertj requested a review from jmayclin February 20, 2025 21:28
@johubertj johubertj marked this pull request as ready for review February 20, 2025 23:25
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@johubertj johubertj requested a review from jmayclin February 24, 2025 22:09
@johubertj johubertj mentioned this pull request Feb 25, 2025
9 tasks
Copy link
Contributor

@jmayclin jmayclin left a comment

Choose a reason for hiding this comment

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

Looks good! But for the sake of documentation, we probably want to add some context to the PR description.

  • What changes are done manually?
  • What changes were automated?
    • What command was used to make the automated changes?
  • Why is this change being made? Why replace pep8?

@johubertj johubertj requested a review from dougch February 25, 2025 22:29
Copy link
Contributor

@dougch dougch left a comment

Choose a reason for hiding this comment

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

Such consistency...nice.
minor nit: can you also cleanup the pep8 config? https://github.com/aws/s2n-tls/blob/main/.pep8

@johubertj johubertj enabled auto-merge February 25, 2025 22:45
@johubertj johubertj disabled auto-merge February 25, 2025 22:46
@johubertj johubertj enabled auto-merge February 25, 2025 22:58
@johubertj johubertj added this pull request to the merge queue Feb 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 26, 2025
@johubertj johubertj added this pull request to the merge queue Feb 26, 2025
Merged via the queue into aws:main with commit ac1d098 Feb 26, 2025
46 checks passed
@johubertj johubertj deleted the test/integv2-formatting-ruff branch February 26, 2025 20:26
@johubertj johubertj changed the title Ruff Formatting and add to CI chore(ruff): apply formatting and integrate into CI Feb 27, 2025
dougch pushed a commit to dougch/s2n-tls that referenced this pull request Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IntegV2 Usability Quest
3 participants