Skip to content

Don't trigger our own deprecation warning #826

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

Conversation

nevinera
Copy link
Contributor

@nevinera nevinera commented Jul 8, 2025

Description

I was getting a lot of deprecation warnings from inside this gem, but when I tracked them down, they were actually just an internal call, and also deprecated internally. Easy fix :-)

Git.open(".") can reproduce the issue on 4.0.1:

  • Git.open calls Git::Base.open
  • which calls the initializer (on line 129)
  • which calls initialize_components (line 157)
  • which calls Git::Index.new with a second positional parameter
  • but Git::Index is a straight subclass of Git::Path, which warns us about the positional parameter

I started to open an issue, and then decided that was silly - this is a trivial fix. (Though I guess I'll need to wait for a gem release to escape the cli noise)

While exercising it, I also noticed that the comments in Rakefile describing how to run individual tests (bin/test tests/units/test_archive.rb) were also outdated, and corrected them.

@nevinera nevinera force-pushed the nevinera/dont-trigger-our-own-deprecation-warning branch from 23cf586 to 087c5fa Compare July 8, 2025 02:30
nevinera added 2 commits July 7, 2025 22:32
(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.
@nevinera nevinera force-pushed the nevinera/dont-trigger-our-own-deprecation-warning branch from 087c5fa to 2aae0d4 Compare July 8, 2025 02:33
@jcouball
Copy link
Member

jcouball commented Jul 8, 2025

@nevinera thanks!

I was meaning to do this myself. Let me review what you have tomorrow (I am in West Coast US) and I'll merge if things look good.

Copy link
Member

@jcouball jcouball left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look great! Super simple so I'll approve and merge right now.

@jcouball jcouball merged commit 07dfab5 into ruby-git:main Jul 8, 2025
7 checks passed
@jcouball
Copy link
Member

jcouball commented Jul 8, 2025

I put this in the test_help.rb as I was doing all the Rubocop integration (which triggered a lot of the deprecations):

# Silence deprecation warnings during tests
Git::Deprecation.behavior = :silence

My next change is to remove this and deal with all the deprecation warnings that the tests produce (there is a lot). A lot of these are tests that need to be updated but I bet some are are (as you characterize) caused by calling our own deprecated stuff.

Thanks for reminding me that this needs to be done.

@nevinera
Copy link
Contributor Author

nevinera commented Jul 8, 2025

Sure, let me know if I can help with that :-)

@nevinera nevinera deleted the nevinera/dont-trigger-our-own-deprecation-warning branch July 8, 2025 11:34
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.

2 participants