quality/blockerbugs
7
0
Fork 0
Code Issues 32 Pull requests 2 Projects 1 Releases Activity Actions

Fix linter problems #317

Manually merged
jgroman merged 3 commits from feature/306-3-ruff-linting into develop 2026-06-22 12:55:22 +00:00
Conversation 6 Commits 3 Files changed 27 +79 -107
jgroman commented 2026-06-10 12:33:23 +00:00
Owner
Copy link
  • Fix ruff check warnings
  • Fix mypy warnings
  • Remove unused imports from requirements
  • Streamline CI jobs

Assisted-by: Claude Code

- Fix ruff check warnings - Fix mypy warnings - Remove unused imports from requirements - Streamline CI jobs Assisted-by: Claude Code
jgroman self-assigned this 2026-06-10 12:33:23 +00:00
Fix linter problems
Some checks failed
Run tests and linters / test (pull_request) Successful in 2m44s
Details
Run tests and linters / lint (pull_request) Failing after 2s
Details
af06f0b3f9 
- Fix ruff check warnings
- Fix mypy warnings
- Remove unused imports from requirements

Assisted-by: Claude Code
Fix mypy target in ci.yml
Some checks failed
Run tests and linters / test (pull_request) Successful in 2m45s
Details
Run tests and linters / lint (pull_request) Failing after 2s
Details
1e67024942
Fix mypy problems found by CI
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m43s
Details
Run tests and linters / lint (pull_request) Successful in 2s
Details
d0b7e7513e
Simplify CI setup since all steps should be passing now
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m49s
Details
AI Code Review / ai-review (pull_request_target) Successful in 22s
Details
ff70b4da7a
jgroman changed title from WIP: Fix linter problems to Fix linter problems 2026-06-10 13:24:40 +00:00
forgejo-actions commented 2026-06-10 13:25:11 +00:00
Copy link

AI Code Review

📋 MR Summary

This merge request resolves various ruff and mypy linting warnings, fixes types and unused dependencies, and simplifies the CI workflow.

  • Key Changes:
    • Replaced standalone mock with unittest.mock across tests.
    • Added proper type hints and type: ignore comments to satisfy mypy.
    • Cleaned up unused dependencies from requirements.txt and requirements-development.txt.
    • Refactored CI pipeline to run tests and linters in a single sequential job instead of parallel/dependent jobs.
  • Impact: CI (.forgejo/workflows/ci.yml), Dependencies (requirements.txt, pyproject.toml), Bot Utilities (blockerbugs/util/forgejo_bot.py), Test Suite (testing/)
  • Risk Level: 🟢 Low - The changes consist mostly of non-functional type hints, linting fixes, and test dependency updates with no core logic alterations.

Detailed Code Review

Overall, these are excellent housekeeping changes. Replacing mock with the standard library unittest.mock reduces external dependencies and aligns with modern Python practices. The typing additions make the codebase more robust and readable.

The only minor point is regarding the CI pipeline modifications. By placing the linting steps after the testing step in the same job, linting feedback will be delayed until the tests finish. If the tests fail, the linters will not run at all. Moving linting to run before tests or in a parallel job would ensure faster feedback on code style issues.

📂 File Reviews

📄 `.forgejo/workflows/ci.yml` - Consolidated linting and testing into sequential steps within the same job.
  • Minor [Suggestion]: Linting steps are executed after tests. This means if tests fail, developers won't get linting feedback, and they must wait for tests to finish to see lint results. It is generally best practice to run linters either before tests or in a parallel job to fail fast.
    • Suggestion: Move the ruff and mypy steps to run before the Run tests step.

✅ Summary

  • Overall Assessment: No critical issues identified. Code style, static typing, and dependencies are cleanly updated.
  • Minor Suggestions:
    • Consider moving linting steps before testing in the CI pipeline to provide faster feedback.

🤖 AI Code Review | Generated with ai-code-review | Model: gemini-3.1-pro-preview

⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.

## AI Code Review ### 📋 MR Summary This merge request resolves various ruff and mypy linting warnings, fixes types and unused dependencies, and simplifies the CI workflow. - **Key Changes:** - Replaced standalone `mock` with `unittest.mock` across tests. - Added proper type hints and `type: ignore` comments to satisfy `mypy`. - Cleaned up unused dependencies from `requirements.txt` and `requirements-development.txt`. - Refactored CI pipeline to run tests and linters in a single sequential job instead of parallel/dependent jobs. - **Impact:** CI (.forgejo/workflows/ci.yml), Dependencies (requirements.txt, pyproject.toml), Bot Utilities (blockerbugs/util/forgejo_bot.py), Test Suite (testing/) - **Risk Level:** 🟢 Low - The changes consist mostly of non-functional type hints, linting fixes, and test dependency updates with no core logic alterations. ### Detailed Code Review Overall, these are excellent housekeeping changes. Replacing `mock` with the standard library `unittest.mock` reduces external dependencies and aligns with modern Python practices. The typing additions make the codebase more robust and readable. The only minor point is regarding the CI pipeline modifications. By placing the linting steps after the testing step in the same job, linting feedback will be delayed until the tests finish. If the tests fail, the linters will not run at all. Moving linting to run before tests or in a parallel job would ensure faster feedback on code style issues. #### 📂 File Reviews <details> <summary><strong>📄 `.forgejo/workflows/ci.yml`</strong> - Consolidated linting and testing into sequential steps within the same job.</summary> - **Minor** [Suggestion]: Linting steps are executed after tests. This means if tests fail, developers won't get linting feedback, and they must wait for tests to finish to see lint results. It is generally best practice to run linters either before tests or in a parallel job to fail fast. - *Suggestion:* Move the `ruff` and `mypy` steps to run before the `Run tests` step. </details> ### ✅ Summary - **Overall Assessment:** No critical issues identified. Code style, static typing, and dependencies are cleanly updated. - **Minor Suggestions:** - Consider moving linting steps before testing in the CI pipeline to provide faster feedback. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) | **Model:** `gemini-3.1-pro-preview` ⚠️ *AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.*
Fix AI review finding: do CI lint jobs first
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m54s
Details
5288d081c8
jgroman commented 2026-06-10 13:53:07 +00:00
Author
Owner
Copy link

Ready for review, I guess

Ready for review, I guess
pyproject.toml: config mypy dir to find files
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m52s
Details
bd7e2f5976
forgejo_bot: use a different approach to satisfy mypy
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m48s
Details
7aae7202ff 
That keeps the original code structure.
kparal force-pushed feature/306-3-ruff-linting from 7aae7202ff
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m48s
Details
to e5c13b8250
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m44s
Details
2026-06-15 10:54:54 +00:00 Compare
drop unneeded ignores
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m52s
Details
586c3cabc4
define mypy files and drop mypy config from setup.cfg
Some checks failed
Run tests and linters / test (pull_request) Failing after 2m30s
Details
81a5deb1a3
sync deps with setup.py, move pytest to devel reqs
Some checks failed
Run tests and linters / test (pull_request) Failing after 2m25s
Details
a27eda93f4
ci: fix mypy
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m50s
Details
1ab292e68d
ci: ./run lint
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m46s
Details
28b71e9933
continue when there are linting errors (test it)
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m46s
Details
c953c14fa0
fix ci
Some checks failed
Run tests and linters / test (pull_request) Failing after 2m48s
Details
09946a7241
ci: ./run tests
Some checks failed
Run tests and linters / test (pull_request) Failing after 2m27s
Details
6674fde55d
fix typo, drop requirements.txt, fold long lines
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m46s
Details
320b521996
kparal force-pushed feature/306-3-ruff-linting from 320b521996
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m46s
Details
to 3fbc4de037
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m48s
Details
2026-06-15 20:03:05 +00:00 Compare
kparal commented 2026-06-15 20:09:08 +00:00
Owner
Copy link

I reworked the existing changes into 3 separate commits, and it's OK to be merged, as far as I'm concerned.

@jgroman WDYT?

I reworked the existing changes into 3 separate commits, and it's OK to be merged, as far as I'm concerned. @jgroman WDYT?
kparal requested changes 2026-06-16 09:39:50 +00:00
Dismissed
.forgejo/workflows/ci.yml
@ -59,3 +59,1 @@
needs: test
steps:
- name: Evaluate lint results
./run test
kparal commented 2026-06-16 09:39:40 +00:00
Owner
Copy link

So I wanted to fix #315 with this, but I forgot that this doesn't likely run python3.11, right? I'll need to fix that or go back to the original invocation.

So I wanted to fix #315 with this, but I forgot that this doesn't likely run python3.11, right? I'll need to fix that or go back to the original invocation.
kparal commented 2026-06-16 09:53:00 +00:00
Owner
Copy link

I was wrong. It's already running in a venv using Python 3.11, so it's using Python 3.11 for pytest as well.

I was wrong. It's already running in a venv using Python 3.11, so it's using Python 3.11 for pytest as well.
kparal marked this conversation as resolved
kparal approved these changes 2026-06-16 09:53:23 +00:00
kparal left a comment
Copy link

Should be fine now, please review as well

Should be fine now, please review as well
jgroman commented 2026-06-22 12:28:18 +00:00
Author
Owner
Copy link

Cannot add myself to reviewers group but LGTM. ✅

Cannot add myself to reviewers group but LGTM. ✅
jgroman manually merged commit 3fbc4de037 into develop 2026-06-22 12:55:22 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
adamwill
kparal
Labels
Clear labels   
Closed As
Duplicate
 Closed with the reason of Duplicate

  
Closed As
Fixed
 Closed with the reason of Fixed

  
Closed As
Invalid
 Closed with the reason of Invalid

  
discussions
issues related to the blocker discussion system in pagure

  
easyfix

   
enhancement
a request for or implementation of an enhancement (not a bug fix)

  
task
issue that isn't a bug fix or enhancement but some kind of admin task

  
ai-review-please
Request AI review for a pull request

  
Backlog Status
Needs Review
 Work items that have been created or updated but need review, refinement, or approval before they can be considered ready for sprint planning.

  
Backlog Status
Ready
 Work items that have been reviewed, refined, and are ready to be committed to a sprint. These items have clear requirements and can be worked on immediately when capacity is available.

  
chore
Work that needs to be done but doesn't add new functionality or fix bugs. Maintenance tasks that keep the system running smoothly and the codebase clean.

  
documentation
Work focused on creating, updating, or improving documentation for users, developers, or system administrators.

  
points
01
 A task that is simple, small, and has very little uncertainty. It should be easy enough for any team member to pick up and complete quickly, without a lot of discussion or research.

  
points
02
 This is a task that is slightly more complex or larger than a "1." It might involve two or three simple steps, but the outcome is still very clear and there's little uncertainty.

  
points
03
 Medium-sized task with some noticeable complexity. It might require a bit more thought or collaboration. The steps are well-defined, but it's not a trivial change.

  
points
05
 A significant piece of work. It has more complexity and a higher degree of uncertainty than smaller tasks. It might require some research, a design discussion, or collaboration across multiple teams.

  
points
08
 Complex task with a lot of uncertainty. It's often the largest size that a team will consider for a single sprint. It likely requires a detailed breakdown and multiple team members to complete.

  
points
13
 Any task that feels like it's a "13" or more is considered an Epic. These are too large and uncertain to be worked on in a single sprint and should be broken down.

  
pr2jira
Sync this pull request to Jira for Agility tracking. Only use for PRs without a corresponding ticket.

  
Priority
Critical
 This is for work that is urgent and of critical importance and so and must be addressed immediately with a higher priority than all other work, even if not originally planned for the current sprint.

  
Priority
High
 This is for work that is critical and needs to be addressed in the next sprint. It's often for urgent bugs, important features, or a major piece of technical debt that is blocking other work.

  
Priority
Low
 This is for work that is good to do but not urgent. It's a candidate for being worked on when there's extra time or when it rises in importance. It can stay in the backlog for a longer period.

  
Priority
Medium
 This is for important work that should be done soon, but isn't as urgent as a high-priority item. It will likely be considered for the next 1-2 sprints.

  
Sprint Status
Blocked
 Work items that cannot progress due to external dependencies, blockers, or issues that need resolution.

  
Sprint Status
Done
 Work items that have been completed, reviewed, and integrated. No further work is needed within the current sprint.

  
Sprint Status
In Progress
 Work items currently being worked on by a team member. Active development, research, or implementation is happening.

  
Sprint Status
Review
 Work items that are complete but need review, testing, or approval before being marked as done.

  
Sprint Status
To Do
 Work items that are committed to the sprint but haven't been started yet. These items are ready to be picked up by team members.

  
Technical Debt
Work that addresses shortcuts, workarounds, or suboptimal implementations that were done previously to meet deadlines. These items improve code quality, maintainability, or performance without adding new features.

  
Work Item
Bug
 Issues where the system is not working as intended or designed. Bugs represent deviations from expected behavior.

  
Work Item
Epic
 Large initiatives that are broken down into multiple smaller work items (user stories, tasks, bugs). Epics represent significant features or projects that span multiple sprints.

  
Work Item
Spike
 Time-boxed research, investigation, or proof-of-concept work to reduce uncertainty or explore solutions. Spikes help inform future development decisions.

  
Work Item
Task
 Used to break down larger user stories into smaller, manageable implementation pieces. They focus on the specific technical steps needed to deliver the user story's value.

  
Work Item
User Story
 User-facing features or functionality that can be framed from a user perspective, describing what someone wants to accomplish and why.

No labels
Closed As
Duplicate
Closed As
Fixed
Closed As
Invalid
discussions
easyfix
enhancement
task
ai-review-please
Backlog Status
Needs Review
Backlog Status
Ready
chore
documentation
points
01
points
02
points
03
points
05
points
08
points
13
pr2jira
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Sprint Status
Blocked
Sprint Status
Done
Sprint Status
In Progress
Sprint Status
Review
Sprint Status
To Do
Technical Debt
Work Item
Bug
Work Item
Epic
Work Item
Spike
Work Item
Task
Work Item
User Story
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
jgroman
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks
#306 Reformat codebase to satisfy CI linter check
quality/blockerbugs
#315 Make ci.yml use devel commands for linting and testing
quality/blockerbugs
Reference
quality/blockerbugs!317
No description provided.