Get rid of annoying empty changelists!...

Get rid of annoying empty changelists!

1) don't save empty changelists to begin with. This guards against an accidental or canceled "gcl change" creating an empty file.
2) create a "clearempties" command which clears all changelists that don't have any files in them.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=22538

[Marc-Antoine Ruel (Google), 2009-07-25 04:13:27]

It's clear the unit tests will fail so I assume you used git-cl.

I'm fine with the new ErrorExit (part 1) but not with the clearempties (part
2).

On Jul 24, 2009 10:28 PM, <estade@chromium.org> wrote:

Reviewers: Marc-Antoine Ruel,

Description:
Get rid of annoying empty changelists!

1) don't save empty changelists to begin with. This guards against an
accidental or canceled "gcl change" creating an empty file.
2) create a "clearempties" command which clears all changelists that
don't have any files in them.


Please review this at http://codereview.chromium.org/160139

SVN Base: svn://chrome-svn/chrome/trunk/tools/depot_tools/

Affected files:
 M     gcl.py


Index: gcl.py
===================================================================
--- gcl.py      (revision 21611)
+++ gcl.py      (working copy)
@@ -681,6 +681,10 @@
      of changenames.
      --> Use 'gcl help try' for more information!

+   gcl clearempties
+      Deletes all changelists that have no files associated with them.
Careful,
+      you can lose your descriptions.
+
   gcl help [command]
      Print this help menu, or help for the given command if it exists.
 """)
@@ -973,7 +977,7 @@
      description = change_info.description

  other_files = GetFilesNotInCL()
-
+
  #Edited files will have a letter for the first character in a string.
  #This regex looks for the presence of that character.
  file_re = re.compile(r"^[a-z].+\Z", re.IGNORECASE)
@@ -1023,6 +1027,11 @@
    status = line[:7]
    file = line[7:]
    new_cl_files.append((status, file))
+
+  if (not len(change_info._files)) and (not change_info.issue) and \
+      (not len(new_description) and (not new_cl_files)):
+    ErrorExit("Empty changelist not saved")
+
  change_info._files = new_cl_files

  change_info.Save()
@@ -1094,6 +1103,16 @@
      print "".join(file)


+def ClearEmptyChangeLists():
+  """Delete all changelists that have no files."""
+  print "\n--- Deleting:"
+  for cl in GetCLs():
+    change_info = ChangeInfo.Load(cl, GetRepositoryRoot(), True, True)
+    if not len(change_info._files):
+      print chane_info.name
+      change_info.Delete()
+
+
 def main(argv=None):
  if argv is None:
    argv = sys.argv
@@ -1151,6 +1170,9 @@
    print '\n'.join(("%s: %s" % (str(k), str(v))
                     for (k,v) in CODEREVIEW_SETTINGS.iteritems()))
    return 0
+  if command == "clearempties":
+    ClearEmptyChangeLists()
+    return 0

  if len(argv) == 2:
    if command == "change":

[darin (slow to review), 2009-07-25 06:00:04]

maybe use the term "delete" instead of "clear" to match the "gcl delete"
command.
-darin



On Fri, Jul 24, 2009 at 7:28 PM, <estade@chromium.org> wrote:

>
> Reviewers: Marc-Antoine Ruel,
>
> Description:
> Get rid of annoying empty changelists!
>
> 1) don't save empty changelists to begin with. This guards against an
> accidental or canceled "gcl change" creating an empty file.
> 2) create a "clearempties" command which clears all changelists that
> don't have any files in them.
>
>
> Please review this at http://codereview.chromium.org/160139
>
> SVN Base: svn://chrome-svn/chrome/trunk/tools/depot_tools/
>
> Affected files:
>  M     gcl.py
>
>
> Index: gcl.py
> ===================================================================
> --- gcl.py      (revision 21611)
> +++ gcl.py      (working copy)
> @@ -681,6 +681,10 @@
>       of changenames.
>       --> Use 'gcl help try' for more information!
>
> +   gcl clearempties
> +      Deletes all changelists that have no files associated with them.
> Careful,
> +      you can lose your descriptions.
> +
>    gcl help [command]
>       Print this help menu, or help for the given command if it exists.
>  """)
> @@ -973,7 +977,7 @@
>       description = change_info.description
>
>   other_files = GetFilesNotInCL()
> -
> +
>   #Edited files will have a letter for the first character in a string.
>   #This regex looks for the presence of that character.
>   file_re = re.compile(r"^[a-z].+\Z", re.IGNORECASE)
> @@ -1023,6 +1027,11 @@
>     status = line[:7]
>     file = line[7:]
>     new_cl_files.append((status, file))
> +
> +  if (not len(change_info._files)) and (not change_info.issue) and \
> +      (not len(new_description) and (not new_cl_files)):
> +    ErrorExit("Empty changelist not saved")
> +
>   change_info._files = new_cl_files
>
>   change_info.Save()
> @@ -1094,6 +1103,16 @@
>       print "".join(file)
>
>
> +def ClearEmptyChangeLists():
> +  """Delete all changelists that have no files."""
> +  print "\n--- Deleting:"
> +  for cl in GetCLs():
> +    change_info = ChangeInfo.Load(cl, GetRepositoryRoot(), True, True)
> +    if not len(change_info._files):
> +      print chane_info.name
> +      change_info.Delete()
> +
> +
>  def main(argv=None):
>   if argv is None:
>     argv = sys.argv
> @@ -1151,6 +1170,9 @@
>     print '\n'.join(("%s: %s" % (str(k), str(v))
>                      for (k,v) in CODEREVIEW_SETTINGS.iteritems()))
>     return 0
> +  if command == "clearempties":
> +    ClearEmptyChangeLists()
> +    return 0
>
>   if len(argv) == 2:
>     if command == "change":
>
>
>

[Evan Stade, 2009-07-26 20:24:28]

On 2009/07/25 04:13:27, Send 2 maruel_chromium.org plz wrote:
> It's clear the unit tests will fail so I assume you used git-cl.

It passes gcl_unittest (I realized that after sending the first CL so it didn't
make it into the email). Is there something else it needs to pass?

> 
> I'm fine with the new ErrorExit (part 1) but not with the clearempties (part
> 2).

Any particular reason? The name, the whole concept, etc?

[M-A Ruel, 2009-08-03 13:57:32]

I'm back.

So, I'd say the concept. You are fixing the problem that generates empty change
list and at the same time to add a command to remove them. If they aren't
created anymore, I don't think the new command is useful anymore. ? Please
convince me. :)


On 2009/07/26 20:24:28, Evan Stade wrote:
> On 2009/07/25 04:13:27, Send 2 maruel_chromium.org plz wrote:
> > It's clear the unit tests will fail so I assume you used git-cl.
> 
> It passes gcl_unittest (I realized that after sending the first CL so it
didn't
> make it into the email). Is there something else it needs to pass?
> 
> > 
> > I'm fine with the new ErrorExit (part 1) but not with the clearempties (part
> > 2).
> 
> Any particular reason? The name, the whole concept, etc?

[Evan Stade, 2009-08-03 17:44:27]

The way I use it, I end up with a lot of old abandoned changelists that have no
files but a non-empty description. Shrug.

[M-A Ruel, 2009-08-05 19:05:32]

lgtm them

[M-A Ruel, 2009-08-05 19:05:53]

On 2009/08/05 19:05:32, Marc-Antoine Ruel wrote:
> lgtm them

then, duh.