Refactor Environment handling: move to BumpCommandLine and add tests by homerooliveira · Pull Request #64 · homerooliveira/Bump

homerooliveira · 2025-03-12T08:43:29Z

No description provided.

gemini-code-assist

Hello @homerooliveira, I'm Gemini Code Assist¹! 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!

Summary of Changes

This pull request focuses on refactoring the environment handling within the BumpCommandLine tool. The primary goal is to move the Environment struct from a separate module into the BumpCommandLine module itself, and to add corresponding unit tests for it. This change aims to improve the modularity and testability of the command-line tool.

Highlights

Environment Refactoring: The Environment struct, which is used for dependency injection in BumpCommand, has been moved from the Environment module to the BumpCommandLine module.
Test Addition: New unit tests have been added specifically for the Environment struct within the BumpCommandLineTests target.
Module Dependency Update: The dependency on the separate Environment module has been removed from BumpCommand.swift, BumpCommandLineIntegrationTests.swift, and BumpCommandTests.swift.

Changelog

Click here to see the changelog

Sources/BumpCommandLine/BumpCommand.swift
- Removed import of the Environment module.
Sources/BumpCommandLine/Environment.swift
- Added a new Environment struct with xcodeProjFinder, xcodeProjWrapper, and logger properties.
- Added a live static instance for default environment configuration.
- Added Decodable conformance for the Environment struct.
Sources/Environment/Environment.swift
- Removed the Environment struct and its related code.
Tests/BumpCommandLineIntegrationTests/BumpCommandIntegrationTests.swift
- Removed import of the Environment module.
Tests/BumpCommandLineTests/BumpCommandTests.swift
- Removed import of the Environment module.
Tests/BumpCommandLineTests/EnvironmentTests.swift
- Updated import statement to @testable import BumpCommandLine.
- Added tests for the Environment struct.

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 is currently in preview and 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 to provide feedback.

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.

Trivia time!

What is the purpose of dependency injection in software development?

Click here for the answer

Dependency injection is a design pattern in which an object receives other objects that it depends on (dependencies). The goal is to separate the responsibility of constructing dependencies from the object itself, promoting loose coupling and making code more testable and maintainable.

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. ↩

gemini-code-assist

Code Review

This pull request refactors the environment handling by moving the Environment struct to BumpCommandLine and adds tests for it. The use of dependency injection with the Environment struct is a good practice for testability. The changes look good overall, but I have a few suggestions for improvement.

Summary of Findings

Redundant Imports: The Environment struct was removed from BumpCommand.swift and BumpCommandLineIntegrationTests.swift, but the import statements were not removed from those files. This can lead to confusion and unnecessary dependencies.

Merge Readiness

The code changes are well-structured and improve testability. However, I recommend addressing the redundant imports before merging. I am unable to directly approve this pull request, so please have another reviewer approve this code before merging.

codecov · 2025-03-12T09:02:17Z

Codecov Report

Attention: Patch coverage is 97.43590% with 1 line in your changes missing coverage. Please review.

Project coverage is 98.93%. Comparing base (92f9e1b) to head (4085645).

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
Sources/BumpCommandLine/Environment.swift	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #64      +/-   ##
==========================================
- Coverage   99.00%   98.93%   -0.07%     
==========================================
  Files          14       14              
  Lines         401      375      -26     
==========================================
- Hits          397      371      -26     
  Misses          4        4

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

homerooliveira · 2025-03-12T09:03:53Z

/gemini review

gemini-code-assist

Code Review

The pull request refactors environment handling by moving the Environment struct to the BumpCommandLine target and adding tests. This improves modularity and testability. The changes look good overall, but there are a few points to consider.

Summary of Findings

Redundant dependency: The Environment target was removed, and its functionality was moved to BumpCommandLine. Consequently, the Environment dependency was removed from BumpCommandLineTests and BumpCommandLineIntegrationTests targets in Package.swift. However, BumpCommandLineTests still depends on FileManagerWrapperMock and XcodeProjWrapperMock, which might be redundant since these mocks are now part of BumpCommandLine's testing scope.
Missing tests for XcodeProjFinder: There are no dedicated tests for XcodeProjFinder. Adding tests for this class would improve the overall test coverage.

Merge Readiness

The pull request is almost ready for merging. Addressing the redundant dependency in BumpCommandLineTests and adding tests for XcodeProjFinder would further improve the quality of the code. I am unable to approve this pull request, and recommend that others review and approve this code before merging.

Package.swift

homerooliveira · 2025-03-12T09:12:20Z

@gemini-code-assist review

gemini-code-assist

Code Review

The pull request refactors the environment handling by moving the Environment struct to the BumpCommandLine target and adding tests for it. This improves the modularity and testability of the code. The changes look good overall, but there are a few minor suggestions for improvement.

Summary of Findings

Redundant Dependencies in Package.swift: The Environment target is removed, and its dependencies are moved to BumpCommandLine. Some test targets still list Environment as a dependency, which is no longer necessary and can be removed.
Test Target Dependencies: The BumpCommandLineTests target in Package.swift has commented-out dependencies (FileManagerWrapperMock, XcodeProjWrapperMock). These should be removed to avoid confusion and ensure the test target only depends on necessary modules.

Merge Readiness

The pull request is almost ready for merging. The identified issues are of medium severity and should be addressed before merging to ensure the project's dependencies are clean and the test setup is clear. I am unable to approve the pull request, and recommend that others review and approve this code before merging.

Package.swift

…ructure

…imports and improve code formatting in tests

…d streamline getter/setter implementations

Copilot

Copilot reviewed 4 out of 24 changed files in this pull request and generated no comments.

Files not reviewed (20)

.swift-format: Language not supported
.vscode/settings.json: Language not supported
Package.swift: Language not supported
Sources/BumpCommandLine/BumpCommand.swift: Language not supported
Sources/BumpCommandLine/Environment.swift: Language not supported
Sources/BumpCore/Bump.swift: Language not supported
Sources/Environment/Environment.swift: Language not supported
Sources/SwiftExtensions/ArrayExtensions.swift: Language not supported
Sources/SwiftExtensions/DictonaryExtensions.swift: Language not supported
Sources/XcodeProjWrapper/BuildConfiguration.swift: Language not supported
Sources/XcodeProjWrapper/Target.swift: Language not supported
Sources/XcodeProjWrapper/XcodeProjWrapper.swift: Language not supported
Tests/BumpCommandLineIntegrationTests/BumpCommandIntegrationTests.swift: Language not supported
Tests/BumpCommandLineTests/BumpCommandTests.swift: Language not supported
Tests/BumpCommandLineTests/EnvironmentTests.swift: Language not supported
Tests/BumpCoreTests/BumpCoreTests.swift: Language not supported
Tests/BumpCoreTests/BumpCoreTests2.swift: Language not supported
Tests/BumpCoreTests/IncrementModeTests.swift: Language not supported
Tests/BumpCoreTests/Mock/XcodeProjWrapperMock+mock.swift: Language not supported
Tests/FileManagerWrapperTests/URLTests.swift: Language not supported

homerooliveira · 2025-04-05T01:22:32Z

/gemini review

gemini-code-assist

Code Review

The pull request refactors the environment handling by moving it to BumpCommandLine and adds tests. The changes look good overall, but there are a few areas that could be improved for clarity and consistency.

Summary of Findings

Redundant imports: Several files have redundant imports that are no longer needed after the refactoring. These should be removed to keep the code clean.
Dependency Injection: The Environment struct is used for dependency injection, which is a good practice. However, the Decodable conformance seems unnecessary and might be misleading.
Code Formatting: There are some minor code formatting issues, such as inconsistent spacing and line breaks, that could be improved for better readability.

Merge Readiness

The pull request is almost ready for merging. Addressing the redundant imports and clarifying the purpose of the Environment struct's Decodable conformance would improve the code quality. I am unable to approve this pull request, and recommend that others review and approve this code before merging. I recommend that the pull request not be merged until the high severity issues are addressed.

Sources/BumpCommandLine/BumpCommand.swift

Sources/BumpCore/Bump.swift

Tests/BumpCommandLineIntegrationTests/BumpCommandIntegrationTests.swift

…e function and improve comments for clarity

…consistency

…uild step

…workflow

… semantics and remove unnecessary deinitializers in BumpCommandTests and XcodeProjFinderTests.

…tter resource management

…x formatting in test files

…ws job

gemini-code-assist bot reviewed Mar 12, 2025

View reviewed changes

Package.swift Show resolved Hide resolved

gemini-code-assist bot reviewed Mar 12, 2025

View reviewed changes

Package.swift Show resolved Hide resolved

homerooliveira added 6 commits April 1, 2025 21:09

Refactor Environment handling: move to BumpCommandLine and add tests

7d199ad

Refactor Package.swift: streamline target dependencies and improve st…

8c181a8

…ructure

Add unit tests for XcodeProjFinder to validate project path discovery

3471037

Remove unused test dependencies from BumpCommandLineTests

c40c605

Add Swift format configuration and update project settings; refactor …

a3fa9ef

…imports and improve code formatting in tests

Format code and remove cast

2cba939

homerooliveira force-pushed the feature/improve-code branch from 8396dec to 2cba939 Compare April 2, 2025 00:12

Refactor BuildConfiguration: remove duplicate property definitions an…

ef8a1fd

…d streamline getter/setter implementations

homerooliveira requested a review from Copilot April 5, 2025 01:18

Copilot AI reviewed Apr 5, 2025

View reviewed changes

gemini-code-assist bot suggested changes Apr 5, 2025

View reviewed changes

Sources/BumpCommandLine/BumpCommand.swift Show resolved Hide resolved

Sources/BumpCore/Bump.swift Outdated Show resolved Hide resolved

Tests/BumpCommandLineIntegrationTests/BumpCommandIntegrationTests.swift Show resolved Hide resolved

homerooliveira added 11 commits April 6, 2025 04:41

Refactor bumping logic: consolidate version bump methods into a singl…

6862ccb

…e function and improve comments for clarity

Add Linux CI job to run Swift tests in a container

b9fc9ac

Update build.yml

cf66491

Update Xcode version in CI configuration and add caching for Linux job

537d740

Update cache action version in CI configuration and adjust paths for …

7e71a44

…consistency

Update Xcode version to 16.3 in CI workflows and clean up cache steps

611dfdb

Enhance Xcode build output and improve caching for Linux CI job

5d7aa72

Refactor CI workflow to simplify test commands and remove redundant b…

b448991

…uild step

Add trailing_comma to disabled_rules in SwiftLint configuration

5a661b8

Remove --strict flag from Periphery scan command in unused code lint …

ed69420

…workflow

Refactor test classes from final class to struct for better value…

47c8404

… semantics and remove unnecessary deinitializers in BumpCommandTests and XcodeProjFinderTests.

homerooliveira added 6 commits April 13, 2025 01:10

Remove unused reset methods from mock classes to streamline code

b084d06

Refactor bumpWithInPlaceTrue test to use withTemporaryCopyFile for be…

8adb280

…tter resource management

Update unused code lint workflow to include --clean-build flag and fi…

d63d6fc

…x formatting in test files

Add Windows CI job to build and run tests in GitHub Actions workflow

60a4e3f

Add caching for derived data in Xcode build workflow and remove Windo…

e01f091

…ws job

Update cache path for Xcode derived data in CI workflow

4085645

homerooliveira self-assigned this Apr 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Environment handling: move to BumpCommandLine and add tests#64

Refactor Environment handling: move to BumpCommandLine and add tests#64
homerooliveira wants to merge 24 commits intomainfrom
feature/improve-code

homerooliveira commented Mar 12, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

codecov bot commented Mar 12, 2025 •

edited

Loading

Uh oh!

homerooliveira commented Mar 12, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

homerooliveira commented Mar 12, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

homerooliveira commented Apr 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

homerooliveira commented Mar 12, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Changelog

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

codecov bot commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

homerooliveira commented Mar 12, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

Uh oh!

homerooliveira commented Mar 12, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

homerooliveira commented Apr 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Mar 12, 2025 •

edited

Loading