"Giuseppe Bilotta" [off-list ref] writes:
And this is precisely the reason why the first time I sent the patch I
did the restyling in the same go: by not touching the
clueless/careless callers and instead bringing gitweb_check_feature to
act in scalar context, it would automatically fix those broken usages,
... which is very bad for reviewability purposes.
By explicitly fixing them before doing the "this will sweep all the
potential bugs under the rug", you can demonstrate a lot more clearly why
these changes are necessary.
Junio C Hamano wrote:
"Giuseppe Bilotta" [off-list ref] writes:
quoted
And this is precisely the reason why the first time I sent the patch I
did the restyling in the same go: by not touching the
clueless/careless callers and instead bringing gitweb_check_feature to
act in scalar context, it would automatically fix those broken usages,
... which is very bad for reviewability purposes.
By explicitly fixing them before doing the "this will sweep all the
potential bugs under the rug", you can demonstrate a lot more clearly why
these changes are necessary.
Well, I think now that it would be best to split this series into
_two_ patches: first Junio's patch fixing (!) gitweb_check_feature()
calls, second original v1 Guiseppe's renaming gitweb_check_feature()
to gitweb_get_feature(), and adding gitweb_check_feature(), and using
gitweb_get_feature() only where needed; optionally fixing "style".
Pure rename IMVHO doesn't look that nice...
--
Jakub Narebski
Poland
Jakub Narebski wrote:
Well, I think now that it would be best to split this series into
_two_ patches: first Junio's patch fixing (!) gitweb_check_feature()
calls, second original v1 Guiseppe's renaming gitweb_check_feature()
to gitweb_get_feature(), and adding gitweb_check_feature(), and using
gitweb_get_feature() only where needed; optionally fixing "style".
It means: first fixup patch, then futureproof patch.
--
Jakub Narebski
Poland