From: Frank Li <lznuaa@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org, msysGit <msysgit@googlegroups.com>
Subject: Re: [msysGit] Using VC build git
Date: Tue, 11 Aug 2009 09:26:21 +0800 [thread overview]
Message-ID: <1976ea660908101826q26faa37ao7920d5cf9d4f53fd@mail.gmail.com> (raw)
In-Reply-To: <alpine.DEB.1.00.0908101609170.8324@intel-tinevez-2-302>
Thank you take care my patch.
I can fix all problems.
This patch is base on v1.6.4 release. My working branch is vc_build
at git://repo.or.cz/tgit.git.
That is actually prototype to approve VC can build git.
The code style is not big problem. I will fix it.
VC build will reuse many msysgit works because msysgit really do many
work at windows porting.
I think the below is most important problem.
1. Where are vcbuild directory put, is it okay under contrib ?
2. How to handle external library, such as zlib? Can use submodule?
2009/8/10, Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> 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
>
>
>
next prev parent reply other threads:[~2009-08-11 1:26 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-10 13:56 Using VC build git Frank Li
2009-08-10 14:07 ` [msysGit] " Johannes Schindelin
2009-08-10 14:29 ` Johannes Schindelin
2009-08-11 1:26 ` Frank Li [this message]
2009-08-12 7:13 ` [msysGit] " Marius Storm-Olsen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1976ea660908101826q26faa37ao7920d5cf9d4f53fd@mail.gmail.com \
--to=lznuaa@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=msysgit@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).