Re: [msysGit] Using VC build git

Hi,

On Mon, 10 Aug 2009, Johannes Schindelin wrote:

To elaborate:  if I see something like this in the --stat:

 contrib/vcbuild/include/zlib.h        | 1357 +++++++++++++++++++++++++++++++++
 contrib/vcbuild/lib/zlib.lib          |  Bin 0 -> 104148 bytes

... I know already that there is no way this can make it into git.git. There just is not.

Also, if the first commit says nothing else than "Rebase to v1.6.4", it is pretty obvious to me that I will not sign off on that (and I just guess that is the very reason you did not sign off on that, either).

Further, putting anything into contrib/ that really belongs into contrib/ is not cutting it, either.

And I am pretty astonished that mingw.[ch] is touched, as VC is definitely not MinGW32.

Changing 1000+ lines of libgit.vcproj in almost every commit is also something I really do not look upon favorably.

Finally, if _no single_ commit message says _anything_ about the reasons why you had to change code outside of vcbuild/, I am only puzzled.

Now, I want to give you a pretty clear idea what has to be done if this is going into 4msysgit.git, ever, because you obviously spent a lot of time on it, and other people want it, too:

- changing "open" to "_open" in mingw.c is a no-no-no.  If you need to use 
  "_open" in VC, then define "open" in the compile flags for mingw.c, but 
  leave code that is not written for VC alone.

- introducing trailing whitespace is usually a sign of not caring enough 
  about clean and neat code.  So just don't do it.

- making link() fail on MinGW32 just to be able to compile it with VC is 
  outright rude against all people who use a free and open compiler 
  instead of a closed one.

- changing an "_snprintf" to "_vsnprintf" in vcbuild/porting.c without 
  anything else is a clear and loud sign that the code before was broken, 
  and that you fix a faulty patch in a later patch.  This is not how we do 
  things in git.git.  We fix the proper patch before the patch series is 
  accepted into mainline.

- violating the coding style -- even if it is in your VC-specific part -- 
  is not an option.  You need to fix the coding style.

- violating the coding style in files that are not VC-specific is not an 
  option at all.  You really need to fix it.

- changing the default editor from "vi" to "notepad2" will break almost 
  every existing Git user's setup.  That is just inexcusable.

Note: these comments are _just for the last_ of your 5 patches.

Just a brief comment on the 4th patch, because I really do not want to spend more time on this round of patches: spelling the opendir() function as "open dir" function in the commit message is misleading, to say the least, and moving code that was added in a previous patch in the same patch series just shows that it was a mistake to begin with. Besides, don't move anything into mingw.c if MinGW32 does not need it.

Hth, Dscho