Daniel P. Berrangé <berra...@redhat.com> writes: > On Tue, Nov 26, 2019 at 11:04:41AM +0100, Markus Armbruster wrote: >> Eduardo Habkost <ehabk...@redhat.com> writes: >> >> > On Mon, Nov 25, 2019 at 08:20:23AM +0100, Markus Armbruster wrote: >> >> Tao Xu <tao3...@intel.com> writes: >> >> >> >> > Add do_strtomul() to convert string according to different suffixes. >> >> > >> >> > Reviewed-by: Eduardo Habkost <ehabk...@redhat.com> >> >> > Signed-off-by: Tao Xu <tao3...@intel.com> >> >> >> >> What's the actual change here? "Refactor" suggests the interfaces stay >> >> the same, only their implementation changes. "Support suffixes list" >> >> suggests some interface supports something new. >> > >> > 1) Parameters added to suffix_mul() (suffix table); >> > 2) do_strtomul() is being extracted from do_strtosz(). >> > >> > do_strtomul() interface and behavior stays the same. >> >> Alright, it's two related changes squashed together (which tends to >> complicate writing good commit messages). 2) is really a refactoring. >> 1) is not: it makes suffix_mul() more flexible. Summarizing 1) and 2) >> as "refactor do_strtosz() to support suffixes list" is confusing, >> because it's about 1), while the interesting part is actually 2). >> >> Moreover, the commit message should state why these two changes are >> useful. It tries, but "to support suffixes list" merely kicks the can >> down the road, because the reader is left to wonder why supporting >> suffix lists is useful. It's actually for use by the next patch. So >> say that. > > Test case additions to test-cutils.c would go a long way to illustrating > what is added & that its working correctly.
Since this patch only prepares for new features, it doesn't need test updates. The next patch adds features, and it does update the tests.
