On Tue, Nov 15, 2016 at 10:40 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Nov 4, 2016 at 11:58 AM, Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
>> The organization of these patches makes sense to me.
>>
>> On 10/20/16 1:14 AM, Michael Paquier wrote:
>>> - 0001, moving all the SHA2 functions to src/common/ and introducing a
>>> PG-like interface. No actual changes here.
>>
>> That's probably alright, although the patch contains a lot more changes
>> than I would imagine for a simple file move. I'll still have to review
>> that in detail.
>
> Even with git diff -M, reviewing 0001 is very difficult. It does
> things that are considerably in excess of what is needed to move these
> files from point A to point B, such as:
>
> - Renaming static functions to have a "pg" prefix.
> - Changing the order of the functions in the file.
> - Renaming an argument called "context" to "cxt".
>
> I think that is a bad plan. I think we should insist that 0001
> content itself with a minimal move of the files changing no more than
> is absolutely necessary. If refactoring is needed, those changes can
> be submitted separately, which will be much easier to review. My
> preliminary judgement is that most of this change is pointless and
> should be reverted.
How do you plug in that with OpenSSL? Are you suggesting to use a set
of undef definitions in the new header in the same way as pgcrypto is
doing, which is rather ugly? Because that's what the deal is about in
this patch.
--
Michael