Skip to content

Conversation

@ByteOtter
Copy link
Contributor

@ByteOtter ByteOtter commented Oct 7, 2025

What this PR does / why we need it:

This PR fixes various typing and some smaller logic issues within the features module.
Fixes include:

  • Raising a RuntimeError before calling any filter function if flavor is None to avoid passing None values
  • Fixing a call to an undefined variable in a ValueError in the Parser class
  • Removing unnecessary Optional type hints in cases where default values are set
  • Explicitly suppressing the return value of functions to mark intent
  • Fixing the syntax of Callable in function signatures
  • Explicitly casting to a networkx.DiGraph in the _exclude_from_filter_set function to avoid accessing successors of Unknown type

While doing these fixes, I noticed that many values are initialized as None and then treated like a regular str type in the code that follows. I tried to assume the most logical approach to determine if these values are allowed to be None in their respective context.

Todo:

  • CName class
  • CName Main module
  • Noneable flavor variable in __main__.py without proper handling

Which issue(s) this PR fixes:
Tracks #152

@ByteOtter ByteOtter force-pushed the lint/fix-features-module branch 2 times, most recently from 76cd00a to dd180df Compare October 8, 2025 13:44
@codecov
Copy link

codecov bot commented Oct 13, 2025

Codecov Report

❌ Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.36%. Comparing base (5126dc7) to head (bf24225).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/gardenlinux/features/__main__.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #218      +/-   ##
==========================================
- Coverage   92.37%   92.36%   -0.01%     
==========================================
  Files          40       40              
  Lines        1822     1821       -1     
==========================================
- Hits         1683     1682       -1     
  Misses        139      139              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ByteOtter ByteOtter marked this pull request as ready for review October 13, 2025 14:52
@ByteOtter ByteOtter requested a review from yeoldegrove October 14, 2025 09:37
@ByteOtter ByteOtter force-pushed the lint/fix-features-module branch from 26780c3 to bf24225 Compare October 15, 2025 09:28
Copy link
Contributor

@yeoldegrove yeoldegrove left a comment

Choose a reason for hiding this comment

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

Changes look ok to me. Good to merge if the linting errors are fixed.

@ByteOtter
Copy link
Contributor Author

Changes look ok to me. Good to merge if the linting errors are fixed.

The current CI failure is a result of errors in other modules. I'm going module by module, next one is flavors.

@ByteOtter ByteOtter merged commit 58af704 into main Oct 15, 2025
9 of 11 checks passed
@ByteOtter ByteOtter deleted the lint/fix-features-module branch October 15, 2025 14:30
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