Re: [msysGit] Using VC build git

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mon, 10 Aug 2009, Johannes Schindelin wrote:

> Please, we have _high_ standards in git.git, and I really do not want to 
> have to take anything to Junio that does not fulfill that standard.

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


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]