[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Patch for making fedabipkgdiff support --dso-only option
Hello Chenxiong,
The patch looks great and I have applied to master, thanks!
I have just made some slight wording/formatting adjustments:
> * tests/runtestfedabipkgdiff.py.in (fedabipkgdiff_mod): Fix a typo
> in initializing this global variable. (test_data_dir): New
> global variable, that is used to reference
> tests/data/test-fedabipkgdiff/.
> (RunAbipkgdiffTest.{test_all_success, test_partial_failure}):
> Fix typo. (Mockglobalconfig.{koji_topdir, dso_only}): New data
> members. (GetPackageLatestBuildTest.{test_get_latest_one,
> test_cannot_find_a_latest_build_with_invalid_distro,
> test_succeed_to_download_a_rpm, test_failed_to_download_a_rpm}):
> Fix typo. (BrewListRPMsTest.test_select_specific_rpms): Fix
> typo. (RunAbipkgdiffWithDSOOnlyOptionTest): New test case
> class.
I fixed this so that it has a proper formatting. That is, referencing
function names (in parenthesis) should be at the beginning of the line.
There must have been some copy/paste issue. Note that in Emacs, there
is a ChangeLog (M-x change-log-mode) that helps greatly to have
ChangeLog correctly formatted. Well, Emacs is just great, I love it :-)
> + * ``--dso-only``
> +
> + Compare ABI of shared libraries only. By default, to compare ABI of all
> + binaries within a RPM.
> +
I amended this slightly, so it now reads like:
* ``--dso-only``
Compares the ABI of shared libraries only. If this option is not
provided, the tool compares the ABI of all ELF binaries found in
the packages.
> parser.add_argument(
> + '--dso-only',
> + required=False,
> + action='store_true',
> + dest='dso_only',
> + help='Compare the ABI of shared libraries only. By default, ABI of all'
> + ' binaries are compared.')
Likewise, I made some slight wording adjustments here. This part now
reads:
parser.add_argument(
'--dso-only',
required=False,
action='store_true',
dest='dso_only',
help='Compare the ABI of shared libraries only. If this option is not '
'provided, the tool compares the ABI of all ELF binaries.')
Thank you again for the patch!
Cheers,
--
Dodji