histedit todo

[Augie Fackler]

In general, I'd like tracking bugs for these opened on the BTS so I can burn through the list. I'm on vacation this week from work, but I plan to work on histedit so it's ready for the release. There's a handful of bugs still in the old histedit bug tracker that I need to research.

I've responded to a few of your comments inline, but in general you seem to have covered the bases. I'd love to review patches for this if you feel inclined to work on it, but bugs in the bugzilla I can work through would also be helpful, as I'm probably going to be spending random fragments of time on this over the next couple of weeks.

On Jul 4, 2012, at 7:40 PM, Mads Kiilerich wrote:

> I haven't used histedit but worked a bit on the test failures it introduced.
> 
> There is in my opinion still some things to do. Here is a badly researched list, covering both minor nit-picking and what might be more important issues:
> 
> There should be some generic help for the extension - some explanation of what it is without assuming git knowledge, a description of what problem it solves and how it solves it and perhaps a small usage example.
> 
> The UI messages should be reviewed, it should use the right (lower) casing and aborts should be single line with a hint. That should be cleaned up before translators start translating.
> 
> 'should' in a note and some debug messages sounds to me like something that is followed by a 'but' and isn't done anyway. The usual style is to just say what we are doing.
> 
> There seems to be some inconsistency in the use of 'rules', 'commands', 'EDITED' and 'editedhistory' in the code and tests. Consistent terminology might help adoption and make it more maintainable.

I'm not sure how terminology in the code/tests matters for adoption? Agree on maintainability.

> If the name of the command line should be changed then it would be better to do it now than later.

Name of the command line changed? Not sure what you mean here. Do you mean you're proposing changing the subcommand name?

> Some variables with unfortunate naming might make maintenance harder: created/created_ and replaced/replaced_.
> 
> It seems like an opener should be used for histedit-last-edit.txt - and perhaps also in other places.

Yes, I think that code predates opener.

> assertions are used. Some of them are so verbose that they look like something the user can hit and thus should have been an abort.

Maybe? I've never hit them.

> 
> hexshort and node.hex(n)[:12] - should probably use node.short instead?

Yep.

> 
> The tests introduce a magic missing-comment.hg - it should have a descriptive name and a description of how it was created ... or be created on the fly.

I can't (IIRC) create that on the fly. It is a change set history that includes a revision with no log message.

> There are some TODO's in the code.
> 
> This introduces the first use of pickle as storage format. Changing the format later on could annoy users. Is that a format we want to commit ourself to? What are the security implications of someone browsing some untrusted repo with the extension enabled?

Migrate to a text format (which we'll treat as binary) with a version number on the first line in case we have to migrate the format.

> 
> /Mads
> 
> 
> 
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel