Skip to content

Fix deprecations and improve warning reporting #830

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

Merged
merged 7 commits into from
Jul 8, 2025

Conversation

jcouball
Copy link
Member

@jcouball jcouball commented Jul 8, 2025

Restore previously deprecated Git::Diff methods and ensure accurate deprecation warnings point to user code.

Update tests to eliminate the use deprecated features (unless testing deprecations themselves).

Make tests that emit a deprecation warning fail to enforce clean test output.

Adjust the deprecation horizon for clarity on future removals.

This partially addreses #829. The only thing left is to test that deprecation warnings are issued in the right circumstances.

jcouball added 7 commits July 8, 2025 10:42
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.
@jcouball jcouball merged commit 7e211d7 into main Jul 8, 2025
7 checks passed
@jcouball jcouball deleted the avoid_deprecation_warnings branch July 8, 2025 21:38
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.

1 participant