Skip to content

Document and Announce the git gem's architectural redesign #834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 65 commits into from

Conversation

jcouball
Copy link
Member

@jcouball jcouball commented Jul 9, 2025

The git gem is undergoing a significant architectural redesign for the upcoming
v5.0.0 release. The current architecture has several design challenges that make it
difficult to maintain and evolve. This redesign aims to address these issues by
introducing a clearer, more robust, and more testable structure.

We have prepared detailed documents outlining the analysis of the current
architecture and the proposed changes. We encourage our community and contributors to
review them:

  1. Analysis of the Current Architecture: A
    breakdown of the existing design and its challenges.
  2. The Proposed Redesign: An overview of the
    new three-layered architecture.
  3. Implementation Plan: The step-by-step
    plan for implementing the redesign.

Your feedback is welcome! Please feel free to open an issue to discuss the proposed
changes.

DON'T PANIC!

While this is a major internal refactoring, our goal is to keep the primary public
API on the main repository object as stable as possible. Most users who rely on
documented methods like g.commit, g.add, and g.status should find the
transition to v5.0.0 straightforward.

The breaking changes will primarily affect users who have been relying on the
internal g.lib accessor, which will be removed as part of this cleanup. For more
details, please see the "Impact on Users" section in the redesign
document
.

jcouball added 30 commits June 6, 2025 14:55
Update the CI builds to build with MRI Ruby 3.2, 3.3,
and 3.4; TruffleRuby 24.2.1; and JRuby 10.0.0.1.

BREAKING CHANGE: Users will need to be on Ruby 3.2 or greater
This partially implements #813

Log data access methods directly on the Log class will return a deprecation warning
since they will be removed in the future.
This pull request refactors the Git::Diff decomposing it into new, more focused
classes, while backward compatibility is maintained via a deprecated facade.
This will match all my other projects to aid copy and pasting between projects.
The Branch, Remote, and Worktree classes all inherit from Path
but do not initialize it. Rather than call super, I am removing the
inheritance. While this could be considered a breaking change,
since the base class was never initialized, objects of these
classes can not make use of the Path methods.
Ignore this offense for the gemspec and for tests since the
DSL for both make it hard to limit the block length.
traylenator and others added 28 commits July 5, 2025 10:23
```
ssh-keygen -t dsa
unknown key type dsa
```
with OpenSSH_9.9p1
Most of this was fixed by excluding tests from this metric. This is
fine since I plan on moving tests to RSpec in the near future.

The other few instances were fixed by refactoring the methods.
Exclude tests from the Metrics/ClassLength Rubocop offfense
since the plan is to rewrite the test suite in RSpec. At that time
many of the Rubocop exclusions will be removed.
This refactoring streamlines the Git::Status class by decomposing
its responsibilities, resulting in cleaner, more focused components:

* `StatusFile`: The inner data responsibilities was moved to its
  own class.
* `StatusFileFactory`: A new private factory was created to
  encapsulate all the logic for executing git commands and parsing
  their output into StatusFile objects.

I think the result is more readable and maintainable code.
The Git::Log class had grown to over 150 lines, making it difficult
to read and maintain. This refactoring simplifies the internal
implementation to improve its structure and reduce its size to under
100 lines, without altering the public API.

The primary motivations were to enhance readability and make the
class easier to extend in the future.

The key changes include:

- Consolidated all query parameters from individual instance
  variables into a single `@options` hash to simplify state
  management.
- Replaced repetitive builder methods with a concise `set_option`
  private helper, reducing code duplication.
- Centralized the command execution logic into a single
  `run_log_if_dirty` method to ensure consistent behavior.
- Simplified the deprecated Enumerable methods by using a helper
  for warnings and safe navigation (`&.`) for cleaner error handling.
- Modernized the nested `Git::Log::Result` class to use
  `Data.define`, leveraging Ruby 3.2+ features to create a more
  concise and immutable result object.
(bin/test prepends the unit_test directory; the quoted calls previously
would fail because it ended up trying to run
`tests/units/test/units/test_archive.rb`)
Use the keyword parameter, not the positional one.
These methods were deprecated with the same thinking that
the Git::Log methods were deprecated. However, where
Git::Log is a query builder, Git::Diff is (mostly) not... it is a
facade over diff, diff_stats, and diff_path_stats.

A problem remains with the Git::Diff class in that if you call
the #path method after retrieving results, the results are not
updated which may cause unexpected results.

I'll consider what changes should be made to the Git::Diff
class at a later date.
ActiveSupport::Deprecation is designed to report the line where the deprecated method
is called (one level up the call stack), not where the warning itself is defined.

The previous implementation triggered warnings from an internal helper method,
causing them to report the location with the Git gem. This commit moves the `warn`
call into the public-facing deprecated method, ensuring the warning correctly points
to the user's code that should be changed.
Correctly report that the current deprecations will be removed
by version 5.0.0 of the git gem.
this is so that non-deprecated use of this gem will not produce
deprecation warnings that the user can't fix.
Several methods, arguments, and behaviors were deprecated
but the tests were not updated to use alternates. This caused a
lot of deprecation warnings when running the tests.
Deprecation warnings should not be ignored.

This is important so that:
* when a user sees a deprecation warning, they can be confident it is coming from
  their code and not this gem
* test output is clean and does not contain noisey deprecation warnings

Tests whose purpose is to test that a deprecation warning is issued in the right
circumstance should mock Git::Deprecation#warn to avoid raising an error.
This class is internal only as well as the classes that inherit
from it (Git::Index and Git::WorkingTree). Remove the
deprecation warning and just remove the deprecated code.
Since Git::Stash#initialize is an internal only method, remove
the deprecation warning and removed the deprecated code.
@jcouball jcouball closed this Jul 9, 2025
@jcouball jcouball deleted the redesign branch July 9, 2025 17:20
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.

4 participants