Add merge request agent #376
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Merge Request Agent designed to process feedback on existing merge requests. The changes include the agent's main logic, workflow definition, supporting tasks, data models, and configuration files.
My review has identified several critical issues in the new agent's workflow logic that would cause runtime errors, such as typos in variable names, incorrect state access, and invalid workflow transitions. There are also some medium-severity issues related to unused imports and incorrect type hints that should be addressed to improve code quality and maintainability. Overall, the structure of the new agent is sound, but these critical bugs need to be fixed before it can function correctly.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new Merge Request Agent capable of processing feedback on existing merge requests. The agent is integrated into the system with new Makefile targets and Docker Compose services. The changes include a new workflow for the agent, new data models for merge request details, and new/refactored tasks for interacting with Git and GitLab. The implementation is solid, but I've found a critical issue in the workflow logic that could lead to failures when committing changes, and a minor issue with a docstring. My review includes suggestions to fix these issues.
Signed-off-by: Nikola Forró <[email protected]>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new merge request agent designed to process and apply feedback from MR comments. The changes are comprehensive, including the agent's logic, workflow steps, supporting tasks, data models, and necessary configuration updates in the Makefile and compose.yaml. The implementation also refactors some existing task functions for better modularity and adds new GitLab tools with corresponding unit tests. The overall approach is sound, but I have identified a couple of areas where robustness and maintainability can be improved.
lbarcziova
left a comment
lbarcziova
left a comment
There was a problem hiding this comment.
very nice work, thanks Nikola! I'm ok with merging as is
| return MergeRequestDetails( | ||
| source_repo=mr.source_project.get_git_urls()["git"], | ||
| source_branch=mr.source_branch, | ||
| target_repo_name=mr.target_project.gitlab_repo.name, | ||
| target_branch=mr.target_branch, | ||
| title=mr.title, | ||
| description=mr.description, | ||
| last_updated_at=mr._raw_pr.updated_at, | ||
| comments=[c for c in comments if f"@{username}" in c.message], | ||
| ) |
There was a problem hiding this comment.
nice and easy start, we can advance this later as needed
| 2. If you updated the spec file, use `rpmlint <PACKAGE>.spec` to validate your changes and fix any new issues. | ||
| 3. Verify any changes to patches by running `centpkg --name=<PACKAGE> --namespace=rpms --release=<DIST_GIT_BRANCH> prep`. | ||
| Repeat as necessary. Do not remove any patches unless all their hunks have been already applied | ||
| to the upstream sources. | ||
| 4. If you removed any patch file references from the spec file (e.g. because they were already applied upstream), | ||
| you must remove all the corresponding patch files from the repository as well. | ||
| 5. Generate a SRPM using `centpkg --name=<PACKAGE> --namespace=rpms --release=<DIST_GIT_BRANCH> srpm`. |
There was a problem hiding this comment.
in future, it would be good to have these shareable (between backport/rebase/mr iteration, we could think of general packaging instructions, wdyt?
| {{comments}} | ||
| You are working on Jira issue {{jira_issue}}. |
There was a problem hiding this comment.
I am wondering if it would be useful giving more context into "what is being fixed", but probablly the merge_request_description is usually descriptive enough?
There was a problem hiding this comment.
In the future we might need to add Jira details. I suppose it depends on how substantial are the requested changes, but if very, then maybe rerunning backport/rebase with feedback from the MR would be a better way.
2 participants
Fixes: https://github.com/packit/jotnar/issues/165