Make all arguments of Milestone.update() optional

ptomato · ptomato · commit 94e8d5ea4e64 · 2013-10-19T21:04:18.000-07:00
This makes it possible to do something like
milestone.update(state='closed') without resetting the title,
which seems closer to the purpose of the underlying GitHub
API.

In addition, previously, milestone.update(title=some_title,
state='closed') would error out because the optional
parameters were not removed before sending the request.
diff --git a/AUTHORS.rst b/AUTHORS.rst
@@ -51,3 +51,5 @@ Contributors
 - Aleksey Ostapenko (@kbakba)
 
 - Vincent Driessen (@nvie)
+
+- Philip Chimento (@ptomato)
diff --git a/github3/issues/milestone.py b/github3/issues/milestone.py
@@ -66,12 +66,13 @@ def iter_labels(self, number=-1):
         return self._iter(int(number), url, Label)
 
     @requires_auth
-    def update(self, title, state='', description='', due_on=''):
+    def update(self, title=None, state=None, description=None, due_on=None):
         """Update this milestone.
 
-        state, description, and due_on are optional
+        All parameters are optional, but it makes no sense to omit all of them
+        at once.
 
-        :param str title: (required), new title of the milestone
+        :param str title: (optional), new title of the milestone
         :param str state: (optional), ('open', 'closed')
         :param str description: (optional)
         :param str due_on: (optional), ISO 8601 time format:
@@ -80,9 +81,10 @@ def update(self, title, state='', description='', due_on=''):
         """
         data = {'title': title, 'state': state,
                 'description': description, 'due_on': due_on}
+        self._remove_none(data)
         json = None
 
-        if title:
+        if data:
             json = self._json(self._patch(self._api, data=dumps(data)), 200)
         if json:
             self._update_(json)
diff --git a/tests/test_issues.py b/tests/test_issues.py
@@ -122,6 +122,8 @@ def test_update(self):
         assert self.m.update(None) is False
         self.not_called()
 
+        assert self.m.update(state='closed')
+
         assert self.m.update('foo', 'closed', ':sparkles:',
                              '2013-12-31T23:59:59Z')
         self.mock_assertions()
