-
Notifications
You must be signed in to change notification settings - Fork 41
Changes made since Xavier's last review #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
0c5718c
Merge pull request #13 from edx-solutions/mjevtic/analytics-fixes
ad86b84
Clean up and reorganization
0887699
Fixed unit tests to run on the newest version of workbench
marjev 1b6dbb6
Updated requirements file
marjev 7920340
Split long lines into two
marjev 39c446a
Added the YAML file for Travis; Added coverage to requirements.txt; D…
marjev 86f99fc
Merge pull request #15 from edx-solutions/mjevtic/SOL-299
8f39a0b
pep8 and pylint should be supported and runable on Travis; changed th…
marjev 403dc44
Merge pull request #16 from edx-solutions/mjevtic/SOL-299
51e1002
Ignore duplicate-code in pylint - it is failing on 5 common lines bet…
75c50bf
Update README to show travis project status
90399e7
Merge pull request #14 from edx-solutions/clean_up
0a83943
Changes as a result of feedback
9d76471
Merge pull request #20 from edx-solutions/mjames/Feedback_Updates
64c7b7a
Added integration tests for studio views; Code clean-up;
marjev 57ccc55
Feedback inspired changes
e9233c4
Merge pull request #22 from edx-solutions/mjames/feedback_updates
marjev 0a7d86e
Merge pull request #23 from edx-solutions/mjevtic/PR-21-review
marjev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| [pep8] | ||
| ignore=E501 | ||
| max_line_length=119 | ||
| exclude=settings |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,240 @@ | ||
| [MASTER] | ||
|
|
||
| # Specify a configuration file. | ||
| #rcfile= | ||
|
|
||
| # Python code to execute, usually for sys.path manipulation such as | ||
| # pygtk.require(). | ||
| #init-hook= | ||
|
|
||
| # Profiled execution. | ||
| profile=no | ||
|
|
||
| # Add files or directories to the blacklist. They should be base names, not | ||
| # paths. | ||
| ignore=migrations | ||
|
|
||
| # Pickle collected data for later comparisons. | ||
| persistent=yes | ||
|
|
||
| # List of plugins (as comma separated values of python modules names) to load, | ||
| # usually to register additional checkers. | ||
| load-plugins= | ||
|
|
||
|
|
||
| [MESSAGES CONTROL] | ||
|
|
||
| # Enable the message, report, category or checker with the given id(s). You can | ||
| # either give multiple identifier separated by comma (,) or put this option | ||
| # multiple time. See also the "--disable" option for examples. | ||
| #enable= | ||
|
|
||
| # Disable the message, report, category or checker with the given id(s). You | ||
| # can either give multiple identifiers separated by comma (,) or put this | ||
| # option multiple times (only on the command line, not in the configuration | ||
| # file where it should appear only once).You can also use "--disable=all" to | ||
| # disable everything first and then reenable specific checks. For example, if | ||
| # you want to run only the similarities checker, you can use "--disable=all | ||
| # --enable=similarities". If you want to run only the classes checker, but have | ||
| # no Warning level messages displayed, use"--disable=all --enable=classes | ||
| # --disable=W" | ||
| # I0011 locally-disabled (module-level pylint overrides) | ||
| # disabling ```duplicate-code``` due to 5 lines that are the same near the top of | ||
| # both xblock files - of course these are similar because it is a core pattern to | ||
| # use - if we separated out into a separate file, we'd have the 5 lines in a separate | ||
| # file and have 2 import statements within each file to get back the stuff - pretty | ||
| # useless warning in this specific case | ||
| disable=I0011,W0232,duplicate-code | ||
|
|
||
|
|
||
| [REPORTS] | ||
|
|
||
| # Set the output format. Available formats are text, parseable, colorized, msvs | ||
| # (visual studio) and html. You can also give a reporter class, eg | ||
| # mypackage.mymodule.MyReporterClass. | ||
| output-format=text | ||
|
|
||
|
|
||
| # Put messages in a separate file for each module / package specified on the | ||
| # command line instead of printing them on stdout. Reports (if any) will be | ||
| # written in a file name "pylint_global.[txt|html]". | ||
| files-output=no | ||
|
|
||
| # Tells whether to display a full report or only the messages | ||
| reports=yes | ||
|
|
||
| # Python expression which should return a note less than 10 (10 is the highest | ||
| # note). You have access to the variables errors warning, statement which | ||
| # respectively contain the number of errors / warnings messages and the total | ||
| # number of statements analyzed. This is used by the global evaluation report | ||
| # (RP0004). | ||
| evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10) | ||
|
|
||
| # Add a comment according to your evaluation note. This is used by the global | ||
| # evaluation report (RP0004). | ||
| comment=no | ||
|
|
||
|
|
||
| [MISCELLANEOUS] | ||
|
|
||
| # List of note tags to take in consideration, separated by a comma. | ||
| notes=FIXME,XXX,TODO | ||
|
|
||
|
|
||
| [VARIABLES] | ||
|
|
||
| # Tells whether we should check for unused import in __init__ files. | ||
| init-import=no | ||
|
|
||
| # A regular expression matching the beginning of the name of dummy variables | ||
| # (i.e. not used). | ||
| dummy-variables-rgx=_|dummy | ||
|
|
||
| # List of additional names supposed to be defined in builtins. Remember that | ||
| # you should avoid to define new builtins when possible. | ||
| additional-builtins= | ||
|
|
||
|
|
||
| [TYPECHECK] | ||
|
|
||
| # Tells whether missing members accessed in mixin class should be ignored. A | ||
| # mixin class is detected if its name ends with "mixin" (case insensitive). | ||
| ignore-mixin-members=yes | ||
|
|
||
| # List of classes names for which member attributes should not be checked | ||
| # (useful for classes with attributes dynamically set). | ||
| ignored-classes=SQLObject | ||
|
|
||
| # When zope mode is activated, add a predefined set of Zope acquired attributes | ||
| # to generated-members. | ||
| zope=no | ||
|
|
||
| # List of members which are set dynamically and missed by pylint inference | ||
| # system, and so shouldn't trigger E0201 when accessed. Python regular | ||
| # expressions are accepted. | ||
| generated-members=REQUEST,acl_users,aq_parent | ||
|
|
||
|
|
||
| [BASIC] | ||
|
|
||
| # Required attributes for module, separated by a comma | ||
| required-attributes= | ||
|
|
||
| # List of builtins function names that should not be used, separated by a comma | ||
| bad-functions=map,filter,apply,input | ||
|
|
||
| # Regular expression which should only match correct function names | ||
| function-rgx=[a-z_][a-z0-9_]{2,50}$ | ||
|
|
||
| # Good variable names which should always be accepted, separated by a comma | ||
| good-names=i,j,k,ex,Run,_ | ||
|
|
||
| # Bad variable names which should always be refused, separated by a comma | ||
| bad-names=foo,bar,baz,toto,tutu,tata | ||
|
|
||
| # Regular expression which should only match functions or classes name which do | ||
| # not require a docstring | ||
| no-docstring-rgx=__.*__ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. General comment on that file - are these the default values that we are including? It might be better to not redefine the defaults - that would make the specificities of this repo more obvious. |
||
|
|
||
|
|
||
| [SIMILARITIES] | ||
|
|
||
| # Minimum lines number of a similarity. | ||
| min-similarity-lines=4 | ||
|
|
||
| # Ignore comments when computing similarities. | ||
| ignore-comments=yes | ||
|
|
||
| # Ignore docstrings when computing similarities. | ||
| ignore-docstrings=yes | ||
|
|
||
| # Ignore imports when computing similarities. | ||
| ignore-imports=no | ||
|
|
||
|
|
||
| [FORMAT] | ||
|
|
||
| # Maximum number of characters on a single line. | ||
| max-line-length=120 | ||
|
|
||
| # Maximum number of lines in a module | ||
| max-module-lines=1000 | ||
|
|
||
| # String used as indentation unit. This is usually " " (4 spaces) or "\t" (1 | ||
| # tab). | ||
| indent-string=' ' | ||
|
|
||
|
|
||
| [IMPORTS] | ||
|
|
||
| # Deprecated modules which should not be used, separated by a comma | ||
| deprecated-modules=regsub,string,TERMIOS,Bastion,rexec | ||
|
|
||
| # Create a graph of every (i.e. internal and external) dependencies in the | ||
| # given file (report RP0402 must not be disabled) | ||
| import-graph= | ||
|
|
||
| # Create a graph of external dependencies in the given file (report RP0402 must | ||
| # not be disabled) | ||
| ext-import-graph= | ||
|
|
||
| # Create a graph of internal dependencies in the given file (report RP0402 must | ||
| # not be disabled) | ||
| int-import-graph= | ||
|
|
||
|
|
||
| [DESIGN] | ||
|
|
||
| # Maximum number of arguments for function / method | ||
| max-args=5 | ||
|
|
||
| # Argument names that match this expression will be ignored. Default to name | ||
| # with leading underscore | ||
| ignored-argument-names=_.* | ||
|
|
||
| # Maximum number of locals for function / method body | ||
| max-locals=15 | ||
|
|
||
| # Maximum number of return / yield for function / method body | ||
| max-returns=6 | ||
|
|
||
| # Maximum number of branch for function / method body | ||
| max-branchs=12 | ||
|
|
||
| # Maximum number of statements in function / method body | ||
| max-statements=50 | ||
|
|
||
| # Maximum number of parents for a class (see R0901). | ||
| max-parents=7 | ||
|
|
||
| # Maximum number of attributes for a class (see R0902). | ||
| max-attributes=7 | ||
|
|
||
| # Minimum number of public methods for a class (see R0903). | ||
| min-public-methods=2 | ||
|
|
||
| # Maximum number of public methods for a class (see R0904). | ||
| max-public-methods=20 | ||
|
|
||
|
|
||
| [CLASSES] | ||
|
|
||
| # List of interface methods to ignore, separated by a comma. This is used for | ||
| # instance to not check methods defines in Zope's Interface base class. | ||
| ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by | ||
|
|
||
| # List of method names used to declare (i.e. assign) instance attributes. | ||
| defining-attr-methods=__init__,__new__,setUp | ||
|
|
||
| # List of valid names for the first argument in a class method. | ||
| valid-classmethod-first-arg=cls | ||
|
|
||
| # List of valid names for the first argument in a metaclass class method. | ||
| valid-metaclass-classmethod-first-arg=mcs | ||
|
|
||
|
|
||
| [EXCEPTIONS] | ||
|
|
||
| # Exceptions that will emit a warning when being caught. Defaults to | ||
| # "Exception" | ||
| overgeneral-exceptions=Exception | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| language: python | ||
|
|
||
| python: | ||
| - "2.7" | ||
|
|
||
| before_install: | ||
| - "export DISPLAY=:99.0" | ||
| - "sh -e /etc/init.d/xvfb start" | ||
|
|
||
| install: | ||
| - "pip install -r requirements.txt" | ||
| - "pip uninstall -y xblock-google-drive && python setup.py sdist && pip install dist/xblock-google-drive-0.1.tar.gz" | ||
|
|
||
| script: | ||
| - DJANGO_SETTINGS_MODULE="settings" nosetests --with-coverage --cover-package="google_drive" --with-django | ||
| - pep8 --config=.pep8 google_drive | ||
| - pylint --rcfile=.pylintrc google_drive --report=no | ||
|
|
||
| after_success: coveralls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,5 @@ | ||
| """ | ||
| Google drive XBlocks | ||
| """ | ||
| from .google_docs import GoogleDocumentBlock | ||
| from .google_calendar import GoogleCalendarBlock | ||
| from .google_calendar import GoogleCalendarBlock |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not disabling this specifically next to the code? That would allow to still catch other code duplications.
Also it would be good to replace the code names like "w0232" by their more explicit version ("no-init").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tried that - code-duplication apparently does not correctly ignore that item. The other 2 are the default settings for pylon, so I have no idea what name to use without looking it up; didn't seem like a big deal seeing as they are the default values.