status-im/PyGithub
1
0
Fork 0
mirror of https://github.com/status-im/PyGithub.git synced 2026-08-31 10:51:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

Review of #189: pep8, copyrights, style, remarks

Browse Source 
For remarks, run: git grep "#189"
They are only my first thoughts while reviewing this pull request,
and should be reviewed themselves.
This commit is contained in:
Vincent Jacques
2013-08-22 12:12:38 +02:00
parent 1787765a61
commit 0f74e4389b
6 changed files with 32 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
github/Consts.py
+7 -5
View File
@@ -21,7 +21,9 @@
# #
################################################################################
# TODO: As of Thu Aug 21 22:40:13 (BJT) Chinese Standard Time 2013
# #189: Line endings should be linux style
# TODO: As of Thu Aug 21 22:40:13 (BJT) Chinese Standard Time 2013
# lots of consts in this project are explict
# should realy round them up and reference them by consts
# EDIT: well, maybe :-)
@@ -30,12 +32,12 @@
# Request Header #
# (Case sensitive) #
################################################################################
REQ_IF_NONE_MATCH = "If-None-Match"
REQ_IF_MODIFIED_SINCE = "If-Modified-Since"
REQ_IF_NONE_MATCH = "If-None-Match"
REQ_IF_MODIFIED_SINCE = "If-Modified-Since"
################################################################################
# Response Header #
# (Lower Case) #
################################################################################
RES_ETAG = "etag"
RES_LAST_MODIFED = "last-modified"
RES_ETAG = "etag"
RES_LAST_MODIFED = "last-modified"
github/GithubException.py
+2 -1
View File
@@ -4,8 +4,8 @@
# #
# Copyright 2012 Vincent Jacques <vincent@vincent-jacques.net> #
# Copyright 2012 Zearin <zearin@gonk.net> #
# Copyright 2013 Vincent Jacques <vincent@vincent-jacques.net> #
# Copyright 2013 AKFish <akfish@gmail.com> #
# Copyright 2013 Vincent Jacques <vincent@vincent-jacques.net> #
# #
# This file is part of PyGithub. http://jacquev6.github.com/PyGithub/ #
# #
@@ -78,6 +78,7 @@ class RateLimitExceededException(GithubException):
Exception raised when the rate limit is exceeded (when Github API replies with a 403 rate limit exceeded HTML status)
"""
class NotModifiedException(GithubException):
"""
Exception raised when conditional request is made to a resoure that has not changed
github/GithubObject.py
+11 -12
View File
@@ -57,14 +57,14 @@ class GithubObject(object):
self._requester = requester
# Make sure headers are signed before any operations on attributes
# Object creatation requires headers as parameter
self._headers = headers;
self._headers = headers
self._initAttributes()
self._storeAndUseAttributes(attributes)
# Ask requester to do some checking, for debug and test purpose
# Since it's most handy to access and kinda all-knowing
if (self.CHECK_AFTER_INIT_FLAG):
requester.check_me(self);
if self.CHECK_AFTER_INIT_FLAG:
requester.check_me(self)
def _storeAndUseAttributes(self, attributes):
self._useAttributes(attributes)
@@ -100,22 +100,21 @@ class GithubObject(object):
else:
return datetime.datetime.strptime(s, "%Y-%m-%dT%H:%M:%SZ")
def save(self, file_name):
def save(self, file_name): # #189: Could we use file-like objects? It would be more "pythonic" than passing filenames.
'''
Save instance to a file
:param file_name: the full path of target file
'''
with open(file_name, 'wb') as f:
pickle.dump(self, f)
pickle.dump(self, f) # #189: This will also save self._requester, and the login/password of the user. She might not appriciate.
# #189: May be better to pickle only self._rawData and self._headers and restore the object with Github.create_from_raw_data
@classmethod
def load(cls, file_name):
@classmethod # #189: Could be a @staticmethod? The docstring would be simpler (no need to explain the type will be same as saved).
def load(cls, file_name): # #189: Could we use file-like objects? It would be more "pythonic" than passing filenames.
'''
Load saved instance from file
:param file_name: the full path to saved file
:rtype: saved instance. The type of loaded instance remains its orginal one and will not be affected by from which derived class the method is called.
:rtype: saved instance. The type of loaded instance remains its orginal one and will not be affected by from which derived class the method is called.
'''
with open(file_name, 'rb') as f:
return pickle.load(f)
@@ -133,7 +132,6 @@ class GithubObject(object):
:type str
'''
return self._headers.get(Consts.RES_LAST_MODIFED)
def update(self):
'''
@@ -156,9 +154,10 @@ class GithubObject(object):
self._storeAndUseAttributes(data)
self.__completed = True
return True
except GithubException.NotModifiedException:
except GithubException.NotModifiedException: # #189: Why raise and catch? Can't we just check?
return False
class NonCompletableGithubObject(GithubObject):
def _completeIfNeeded(self):
pass
github/Requester.py
+2
View File
@@ -290,6 +290,8 @@ class Requester:
requestHeaders["Authorization"] = self.__authorizationHeader
def __conditional(self, requestHeaders, parameters):
# #189: Why pass etag and last_modified by param "parameters"?
# #189: May be better to add a specific param "headers" to methods requestFoobar?
etag = parameters.get(Consts.REQ_IF_NONE_MATCH)
last_modified = parameters.get(Consts.REQ_IF_MODIFIED_SINCE)
if etag is not None:
github/tests/ConditionalRequestUpdate.py
+5 -1
View File
@@ -21,13 +21,17 @@
# #
################################################################################
# #189: Line endings should be linux style
import Framework
import github
class ConditionalRequestUpdate(Framework.TestCase):
def setUp(self):
Framework.TestCase.setUp(self)
self.repo = self.g.get_repo("akfish/PyGithub")
self.repo = self.g.get_repo("akfish/PyGithub")
# #189: Let's separate this assert in its own test method, remove it from setUp.
# Not updated
self.assertFalse(self.repo.update(), msg="The repo is not changes. But update() != False")
github/tests/_record_.py
+5 -2
View File
@@ -28,6 +28,9 @@ import github.tests.Framework
import github.tests.AllTests
# #189: This seems equivalent to "python -m github.tests ClassName.methodName --record"
def main(argv):
if len(argv) < 2:
print "Run sepecified test in record mode."
@@ -35,10 +38,10 @@ def main(argv):
print "_record_.py [module_name] [other_arg] ..."
print " e.g. _record_.py AllTests"
return
github.tests.Framework.activateRecordMode()
module_to_run = argv.pop(1)
print "module: " + module_to_run
print "argv: ",
print argv