Hi Ondrej,
Your patch seems to be built on top of Andi's Bazaar patch. I have
couple of quick comments:
1) It will be better if GuessBase() is checked in with the existing
subversion support alone. Then, it can be modified as needed.
2) You only modified Subversion's ValidateOptions() and Bazaar's.
OTOH, I think this type of checking becomes unwieldy as more and more
VCS types are added.
BTW, http://codereview.appspot.com/2878/diff/1/2 is giving me the
error "Bad content. Try to upload again.".
Thanks,
Raghu
On Tue, Aug 12, 2008 at 8:22 PM, Raghuram Devarakonda
<drag...@gmail.com> wrote:
>
> On Tue, Aug 12, 2008 at 2:01 PM, Ondrej Certik <ond...@certik.cz> wrote:
>>
>> Hi,
>>
>> I just wrote a patch that adds support for Mercurial:
>>
>> http://codereview.appspot.com/2878
>
> Hi Ondrej,
>
> Your patch seems to be built on top of Andi's Bazaar patch. I have
> couple of quick comments:
>
> 1) It will be better if GuessBase() is checked in with the existing
> subversion support alone. Then, it can be modified as needed.
> 2) You only modified Subversion's ValidateOptions() and Bazaar's.
> OTOH, I think this type of checking becomes unwieldy as more and more
> VCS types are added.
Thanks for the review. I agree, that this should be sorted out.
>
> BTW, http://codereview.appspot.com/2878/diff/1/2 is giving me the
> error "Bad content. Try to upload again.".
This is a serious problem, which I don't understand. I uploaded with
my modified upload.py from mercurial. I thought the problem could be
with the "a/" and "b/" in file paths. So I fixed upload.py and
uploaded something here:
http://codereview.appspot.com/2897
As you can see, the patch seems ok to me:
http://codereview.appspot.com/2897/patch/1/2
but when you go:
http://codereview.appspot.com/2897/diff/1/2
it says:
Bad content. Try to upload again.
So I guess the rietveld doesn't like something on it. I need to figure this out.
Ondrej