On 05/15/2012 03:49 PM, Thomas Gummerer wrote:
Thanks again for your feedback. I've refactored the code again,
thanks to your suggestions.
Good. I'll try to review the new version as soon as possible.
I suggest that you apply the same kinds of cleanups to
git-convert-index.py (which I personally haven't looked at yet at all).
If you want my feedback on that script, please let me know when you
think it is ready.
> If I'm correct it's fine to have the
compiled structs global?
Yes, that's OK because they are constants so there is no risk of them
propagating side-effects. (Of course, Python doesn't enforce the
constness of identifiers, but by convention ALL_CAPS identifiers are
constants and it would be an obvious no-no to modify one.)
A real Pythonic solution would probably encapsulate all of your code
(including the constants) in classes. But since you will eventually
translate the code to C, I don't think that step is crucial. If, on the
other hand, you propose to include Python scripts in the git
distribution, then making them Pythonic would definitely be on the agenda.
What is your plan for testing this code, and later the C version?
For example, you might want to have a suite of index files with
various contents, and compare the "git ls-files --debug" output with
the output that is expected. How would you create index files like
this? Via git commands? Or should one of your Python scripts be
taught how to do it?
I thought of using real world examples for this, for example the
WebKit index, which is pretty large, and some others, for example the
git index and the linux kernel index.
It is good to do such manual tests, but you probably also want small,
hand-constructed, automatable tests to make sure that all of the
codepaths are exercised. For example,
* A directory containing only files
* A directory containing only subdirectories
* A mixed directory whose last element is a file / last element is a
subdirectory
* Various types of conflicts
...etc.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
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