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

Fix: Don't double unwrap config #46

Merged
merged 3 commits into from
Nov 2, 2023
Merged

Conversation

grahamtt
Copy link
Contributor

Config values passed via pyproject.toml weren't being found because the config was being unwrapped twice.

What

Remove an unnecessary call to get_ruff_config.

Why

The extra call was preventing config values from being applied.

References

N/A

Checklist

  • [ X ] Tests

Config values passed via pyproject.toml weren't being
found because the config was being xx
@grahamtt grahamtt requested a review from a team as a code owner October 26, 2023 21:20
@greenbonebot greenbonebot enabled auto-merge (rebase) October 26, 2023 21:21
@github-actions
Copy link

Conventional Commits Report

😢 No conventional commits found.

👉 Learn more about the conventional commits usage at Greenbone.

@bjoernricks
Copy link
Contributor

Hi,

I had a look into the config code and I am really sure you get the root of the pyproject.toml config. Thus tools.autohooks.ruff still must be used and I am not sure why it didn't work for you.

@grahamtt
Copy link
Contributor Author

The issue is we're calling get_ruff_config at line 40 and at line 61. We only need one of those. I opted to keep the latter.

@bjoernricks
Copy link
Contributor

The issue is we're calling get_ruff_config at line 40 and at line 61. We only need one of those. I opted to keep the latter.

Oh why didn't I see that 🤔 ? Thanks for taking care!

@bjoernricks
Copy link
Contributor

The tests are currently failing. Could you fix them? Afterwards I would love to create a new release with your changes.

Config values passed via pyproject.toml weren't being
found because the config was being xx
auto-merge was automatically disabled November 2, 2023 02:37

Head branch was pushed to by a user without write access

@greenbonebot greenbonebot enabled auto-merge (rebase) November 2, 2023 02:38
@y0urself y0urself changed the title Don't double unwrap config Fix: Don't double unwrap config Nov 2, 2023
@y0urself y0urself enabled auto-merge (squash) November 2, 2023 06:10
Copy link

codecov bot commented Nov 2, 2023

Codecov Report

Merging #46 (82b8d28) into main (debfaec) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #46      +/-   ##
==========================================
+ Coverage   97.29%   97.32%   +0.02%     
==========================================
  Files           3        3              
  Lines         111      112       +1     
==========================================
+ Hits          108      109       +1     
  Misses          3        3              
Files Coverage Δ
autohooks/plugins/ruff/ruff.py 93.75% <ø> (-0.13%) ⬇️
tests/test_ruff.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@y0urself y0urself merged commit 401d42f into greenbone:main Nov 2, 2023
16 checks passed
@bjoernricks
Copy link
Contributor

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants