Some users have been complaining after Focal release about some ruby 2.7 warnings popping up when vagrant is called via CLI. Debian is ahead of us and the newer version has two patches to fixes those warnings:
Since focal is out, if you want to add them via SRU you'll also need a bug report with the SRU template filled out in its description. Warning fixes can be hard to justify as impactful, however likely you can re-use the arguments provided by the users requesting the change. IOW, what about the warnings is bothering them?
(Fwiw, I despise warnings myself, when they don't flag actual problems that need dealt with. So kudos for tackling them.)
The patches also need origin and perhaps other DEP3 metadata included.
This is more personal style, but I would also suggest describing them more specific than "warnings fixes" and "more warnings fixes". I generally also like to make each patch address a particular class of warning, and say what's being done to inhibit them.
A few bits of advice on SRUs (nothing needing changed for this one, but to keep in mind next time):
With the Potential Regression section, one thing I try to do beyond just describing the *likelihood* of a regression, is to think about what a regression would look like. Because, of course the potential should always be Low - if it were high you obviously wouldn't consider SRUing the bug. But a more realistic scenario is that someone updates to your fix, and something completely unrelated breaks, and they wonder if your fix caused their break. By describing what a regression would look like, you can give bug reporters and triagers some clue that actually no, their problem is unlikely to be Lucas' fault, go look elsewhere. ;-)
I've also found by thinking from this direction once and a while I realize there was some angle to the bug I didn't adequately cover, and thus ended up with a better fix overall. So it's a good practice. I think Martin Pitt taught me this way back when.
Also, in test cases, I like to also include directions to install the fixed version, and then show what the output is in that case, for comparison. For this bug the test case is simple enough that's not important. One thing I might do is to trim the usage output, since its text is unnecessary for validating the bug, e.g.:
# Broken behavior:
$ apt install vagrant
$ vagrant --help > /dev/null
NOTE: Gem::Specification.default_specifications_dir is deprecated; use Gem.default_specifications_dir instead. It will be removed on or after 2020-02-01.
Gem::Specification.default_specifications_dir called from /usr/share/rubygems-integration/all/gems/vagrant-2.2.6/lib/vagrant/bundler.rb:428.
/usr/share/rubygems-integration/all/gems/vagrant-2.2.6/lib/vagrant/errors.rb:103: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/usr/share/rubygems-integration/all/gems/i18n-1.8.2/lib/i18n.rb:195: warning: The called method `t' is defined here
(eval):3: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/usr/share/rubygems-integration/all/gems/vagrant-2.2.6/lib/vagrant/ui.rb:223: warning: The called method `say' is defined here
$
My rationale for being a bit pedagogic with the test case is because the more "paint by numbers" it is, and easier it is to use, the more likely someone else will bother spending time validating the fix, and the more smoothly and swiftly it'll go through the SRU process. It also can help future bug reporters and triagers to quickly check that their unrelated issue is not caused by this fix.
> A few bits of advice on SRUs (nothing needing changed for this one, but to
> keep in mind next time):
>
> With the Potential Regression section, one thing I try to do beyond just
> describing the *likelihood* of a regression, is to think about what a
> regression would look like. Because, of course the potential should always be
> Low - if it were high you obviously wouldn't consider SRUing the bug. But a
> more realistic scenario is that someone updates to your fix, and something
> completely unrelated breaks, and they wonder if your fix caused their break.
> By describing what a regression would look like, you can give bug reporters
> and triagers some clue that actually no, their problem is unlikely to be
> Lucas' fault, go look elsewhere. ;-)
I see your point and it makes sense. I've checked SRUs proposed by other people but apparently some people write this section with a different perspective. Anyway, it is a good advice, thanks!
> Also, in test cases, I like to also include directions to install the fixed
> version, and then show what the output is in that case, for comparison. For
> this bug the test case is simple enough that's not important. One thing I
> might do is to trim the usage output, since its text is unnecessary for
> validating the bug, e.g.:
You are right, it'd make the warning more clear in this case.
New changelog entries:
* Merge from Debian unstable. Remaining changes:
- Drop integration-test autopkgtest as it depends on access to the Debian
archive.
New changelog entries:
* New upstream version 2.2.6+dfsg
* Refresh patches
* Bump dependency on ruby-vagrant-cloud to >= 2.0.3
* autopkgtest: don't try running a box under /tmp
* Bump Standards-Version to 4.4.1; no changes needed
* autopkgtest: drop obsolete resstriction needs-recommends
* debian/rules: drop README.md files outside of doc dirs
* autopkgtest smoke-test: use vagrant-libvirt
New changelog entries:
* New upstream version 2.2.3+dfsg
- adds support for Virtualbox 6 (Closes: #921036)
* Rebase patches
* Update packaging with new run of dh-make-ruby
- Bump debhelper compat to 11
- Update dependencies
- Bump Standards-Version to 4.3.0, no changes needed otherwise
- Update Homepage: to an https:// URL
- Add Rules-Require-Root: no
- wrap-and-sort
New changelog entries:
[ Antonio Terceiro ]
* New upstream release
* Refresh patches
* Add missing dependency on rsync (Closes: #896660)
* Bump dependency on ruby-net-ssh to >= 1:5, as vagrant now uses features
that are not present before that version.
[ Jelmer Vernooij ]
* Use secure copyright file specification URI.
New changelog entries:
* New upstream version 2.1.2+dfsg (Closes: #906444)
* Refresh patches
* debian/{copyright, control}: point to https://github.com/hashicorp
* Change dependency from bsdtar to libarchive-tools (Closes: #900489)
* Bump Standards-Version to 4.2.0; no changes needed
New changelog entries:
* New upstream release.
- Removes usage of setting name deprecated by ruby-net-ssh (Closes: #884104)
* debian/tests/integration-test: make sure box pulled for testing is a
Debian one to avoid failures on non-Debian hosts (Closes: #889174)
* Refresh patches:
- 0001-Disable-Checkpoint.patch: removed as checkpoint integration was
dropped upstream
- 0007-Virtualbox-5.2-support.patch: removed, applied upstream
- Renumbered remaining patches
* Bump Standards-Version to 4.1.3; no changes needed
* Change Maintainer to the Debian Ruby Team
* Point Vcs-* to salsa.debian.org
* debian/rules: check dependencies during the build
- added dependencies as build dependencies
* debian/vagrant.lintian-overrides: override warning about shebang in
upstream bash completion file
Since focal is out, if you want to add them via SRU you'll also need a bug report with the SRU template filled out in its description. Warning fixes can be hard to justify as impactful, however likely you can re-use the arguments provided by the users requesting the change. IOW, what about the warnings is bothering them?
(Fwiw, I despise warnings myself, when they don't flag actual problems that need dealt with. So kudos for tackling them.)
The patches also need origin and perhaps other DEP3 metadata included.
This is more personal style, but I would also suggest describing them more specific than "warnings fixes" and "more warnings fixes". I generally also like to make each patch address a particular class of warning, and say what's being done to inhibit them.