feat: remove legacy monorepo-related code #1822
Conversation
|
Skipping CI for Draft Pull Request. |
|
/test all |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @Camila-B, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request refactors the codebase by systematically removing all legacy code paths and functionalities associated with the mono-repository mode. This change streamlines the application by eliminating deprecated features, simplifying status reporting, and removing related utility functions and tests, thereby focusing the codebase solely on multi-repository configurations.
Highlights
- Deprecation of Mono-Repo Logic: Core functions in cmd/nomos/status/client.go and cmd/nomos/status/cluster_state.go that handled mono-repository specific logic, status calculations, and Git configurations have been entirely removed.
- Streamlined Status and Version Reporting: The cmd/nomos/status/status.go and cmd/nomos/version/version.go files have been updated to no longer track or report on mono-repository clusters, simplifying their internal logic and output.
- Removal of Legacy Utilities and Tests: The util.MonoRepoNotice function and its associated strings import have been deleted from cmd/nomos/util/util.go, along with the TestRepoState_MonoRepoStatus test case and v1 import in cmd/nomos/status/cluster_state_test.go, ensuring that no remnants of the legacy mode remain.
- Bug Report Simplification: The bug report generation logic in pkg/bugreport/bugreport.go no longer includes checks or notices related to the enableMultiRepo flag, further cleaning up the codebase.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request is a refactoring to remove legacy code related to the mono-repo mode. The changes are spread across multiple files in the nomos command and its utility packages. The deletions and simplifications appear correct and consistent with the goal of removing mono-repo support. I have one minor suggestion to update a comment that has become outdated due to the refactoring.
Camila-B
force-pushed
the
nomos-monorepo
branch
from
e94956c to
c31e762
Compare
|
/retest |
Camila-B
force-pushed
the
nomos-monorepo
branch
from
c31e762 to
896e6ae
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a great refactoring that successfully removes the legacy monorepo-related code from the nomos CLI. The changes are clean and significantly reduce the codebase by removing unused functions, tests, and logic across multiple files. I've only found a couple of minor opportunities to improve comments for clarity after the refactoring. Overall, excellent work!
Camila-B
force-pushed
the
nomos-monorepo
branch
from
896e6ae to
11a29ad
Compare
Camila-B
changed the title
Camila-B
force-pushed
the
nomos-monorepo
branch
3 times, most recently
from
9699c8c to
11a29ad
Compare
Camila-B
changed the title
Camila-B
force-pushed
the
nomos-monorepo
branch
from
11a29ad to
30a46cd
Compare
|
@sdowell thanks! I updated the type |
| fmt.Println("ConfigManagement parsing error", err) | ||
| } | ||
| if !isMulti { | ||
| util.MonoRepoNotice(os.Stdout, currentk8sContext) |
There was a problem hiding this comment.
On further consideration I think the MonoRepoNotice is still valid, but the existing logic needs some refinement. The notice should only be printed if ConfigManagement is present and configured for mono repo mode.
The main thing we care about changing here is the red herring of the MonoRepoNotice when Config Sync is installed standalone (ConfigManagement not present)
Camila-B
force-pushed
the
nomos-monorepo
branch
from
30a46cd to
5afaf0d
Compare
Camila-B
force-pushed
the
nomos-monorepo
branch
from
5afaf0d to
f8f1e25
Compare
|
@Camila-B: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
MonoRepoNoticewhich prints out a list of clusters that are still using monorepo mode.