Mercurial support

0 views
Skip to first unread message

Ondrej Certik

unread,
Aug 12, 2008, 2:01:14 PM8/12/08
to coderevie...@googlegroups.com
Hi,

I just wrote a patch that adds support for Mercurial:

http://codereview.appspot.com/2878

Ondrej

Raghuram Devarakonda

unread,
Aug 12, 2008, 2:22:49 PM8/12/08
to coderevie...@googlegroups.com

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

Ondrej Certik

unread,
Aug 12, 2008, 3:04:09 PM8/12/08
to coderevie...@googlegroups.com
Hi Raghuram,

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

Reply all
Reply to author
Forward
0 new messages