On Mon, Mar 30, 2009 at 10:35:32AM +0800, Edmund Wong wrote:
> Here's final patch. I appreciate your patience, guys.
Don't worry! I appreciate you stubbornness to get this
done properly :)
> (I hope the log is ok. I believe it is a follow up to
> r36302, as per the thread "format file in working copies").
>
> Log as follows:
>
> [[[
> Followup to r36302.
>
> Modified change-svn-wc-format.py to create or update
> format file in the .svn folder.
>
> * tools/client-side/change-svn-wc-format.py
> (WCFormatConverter: write_dir_format): Added a check
> if the entries file was parsed properly. If so,
> the format file is also created (if it does not
> exist in the .svn folder) or updated with the
> given format_nbr value as given by the
> write_dir_format parameter.
>
> (class Format): Added new class to take
You don't need to mention the type of the things in parens,
i.e. just saying "(Format): New class to take ..." etc. is enough.
> care of the .svn/format file.
>
> Patch by: Edmund Wong <edmund_at_belfordhk.com>
> Reviewed by: stsp <stsp_at_elego.de>
> arfrever <arfrever_at_gentoo.org>
> Daniel Shahaf <d.s_at_daniel.shahaf.name>
All reviewers are committers (listed in trunk/COMMITTERS).
For committers, the tigris.org account name is enough.
Reviewed by: stsp
arfrever
danielsh
But don't worry, people committing patches will usually spot
and fix this before committing.
> Index: change-svn-wc-format.py
> ===================================================================
> --- change-svn-wc-format.py (revision 36847)
> +++ change-svn-wc-format.py (working copy)
> @@ -84,7 +84,7 @@
> if self.verbosity:
> print("Processing directory '%s'" % dirname)
> entries = Entries(os.path.join(dirname, get_adm_dir(), "entries"))
> -
> + entries_parsed = True
The indentation of the line above seems to be off by one character.
Won't python complain about this?
> if self.verbosity:
> print("Parsing file '%s'" % entries.path)
> try:
> @@ -94,7 +94,17 @@
> raise
> sys.stderr.write("%s, skipping\n" % e)
> sys.stderr.flush()
> + entries_parsed = False
Same for line above, I think we need two chars of indentation, not one.
>
And the whole following block also seems to be off by one char,
it is at the same level as your
+ entries_parsed = True
line above:
> + if entries_parsed:
> + format = Format(os.path.join(dirname, get_adm_dir(), "format"))
> + if self.verbosity:
> + print("Updating file '%s'" % format.path)
> + format.write_format(format_nbr, self.verbosity)
> + else:
> + if self.verbosity:
> + print("Skipping file '%s'" % format.path)
> +
The context has the correct indentation levels:
> if self.verbosity:
> print("Checking whether WC format can be converted")
> try:
> @@ -298,7 +308,26 @@
> rep += "[%s] %s\n" % (Entries.entry_fields[i], self.fields[i])
> return rep
>
This seems to be off by one char again (maybe something weird happened
to the mail you sent???):
> +class Format:
> + """Represents a .svn/format file."""
>
> + def __init__(self, path):
> + self.path = path
> +
> + def write_format(self, format_nbr, verbosity=0):
> + format_string = '%d\n'
> + if os.path.exists(self.path):
> + if verbosity >= 1:
> + print("%s will be updated." % self.path)
> + os.chmod(self.path,0600)
> + else:
> + if verbosity >= 1:
> + print("%s does not exist, creating it." % self.path)
> + format = open(self.path, "w")
> + format.write(format_string % format_nbr)
> + format.close()
> + os.chmod(self.path, 0400)
> +
> class LocalException(Exception):
> """Root of local exception class hierarchy."""
> pass
Other than the indentation everything looks good to me now!
Thanks,
Stefan
Received on 2009-03-30 15:23:57 CEST