Added Travis CI Configuration #94

brhoades · 2018-01-18T23:31:00Z

WIP: Not sure if this is done, I need Travis to pick up this branch first.
Cool, pyflakes works. I'll add flake8 and do linting for an hour.

Fixes #81 and fixes #80

LinuxMercedes · 2018-01-18T23:32:35Z

.travis.yml

+install:
+  - pip install -r requirements.txt
+script:
+  - pyflakes assigner/**/*.py assigner/*.py


This should just be pyflakes assigner -- pyflakes (and pylint) take the name of a python module to check.

Thanks, this isn't like flake8 at all.

LinuxMercedes · 2018-01-18T23:52:40Z

.travis.yml

+  - pip install -r requirements.txt
+script:
+  - pyflakes assigner
+  - pylint assigner


Let's leave pylint commented out until I fix all the remaining warnings! I'll start working on that but it'll be a bit (got some 5201 stuff to do first).

Already did it, sorry. It's soothing as hell.

You're a madman!

name was being shadowed by an instance var.

brhoades · 2018-01-19T00:51:04Z

@LinuxMercedes I'd appreciate it if you could run this branch through a few tests on gitlabs. I can only confirm so much.

brhoades · 2018-01-19T00:52:09Z

assigner/baserepo.py

        """Make a Gitlab POST request"""
+        if not params:


I'd love a better way to do this. Per pylint, default parameters that are objects are shared across all invocations.

That is true, although in this case it's not a problem and perhaps even cuts down on the amount of garbage allocated (although frankly I'm really not worried about that for Assigner 😄).

The reason why it's not a problem is that this function doesn't carry state in the default params or payload objects -- the default payload object just stays empty, and the default params object just has the private_token key which is updated every time around. I suppose we could worry about requests.post modifying them but it doesn't seem to right now.

In short: I'm ok with telling pylint to ignore that error in these cases.

Awesome, I will go back and update this.

brhoades · 2018-01-19T00:54:13Z

assigner/baserepo.py

@@ -422,7 +456,7 @@ def new(cls, base_repo, semester, section, username, token):
        return cls.from_url(result["http_url_to_repo"], token)

    @classmethod
-    def name(cls, semester, section, assignment, user):
+    def build_name(cls, semester, section, assignment, user):


This method had some weird conflict with Repo.name set in init. I highly doubt this ever caused a bug.

It wouldn't work anyway... functions were missing args.

brhoades · 2018-01-19T00:58:07Z

requirements.txt

+pylint==1.8.1
+wrapt==1.10.11
+python-utils==2.2.0
+tqdm==4.19.5


This was missing :|

Ah dang, can you add a step in the release steps in CONTRIBUTING.md for updating requirements.txt?

brhoades · 2018-01-19T00:58:33Z

assigner/baserepo.py

@@ -436,18 +470,3 @@ def name(cls, semester, section, assignment, user):
    def push(self, base_repo, branch="master"):
        """Push base_repo code to this repo"""
        base_repo.push_to(self, branch)
-
-
-if __name__ == "__main__":


I assumed this wasn't being used and was for testing / from the old one file assigner.
@LinuxMercedes @michaelwisely confirm?

Yep, that all can go!

LinuxMercedes · 2018-01-19T05:07:36Z

assigner/commands/get.py

-                            result.ref.name, str(result.old_commit)[:8], str(result.ref.commit)[:8]
-                        )])
+                        output.add_row([
+                            row, sec, sid, name, "{}: {} -> {}".format(


Every time I see this line, the {}: {} -> {} bit fills me with hope that I've somehow stumbled into a haskell codebase

Yeah, it looks like a js lambda for me :D

LinuxMercedes · 2018-01-19T05:15:59Z

Holy crap this is nice! I'll run it through its paces tomorrow morning.

LinuxMercedes · 2018-01-19T05:16:56Z

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## Devel

+- Removed lint, removed old baserepo standalone code, and added travis config.


Want to split this into two things? (lint/old code & travis)

I'm no stickler for "one change per PR"

LinuxMercedes · 2018-01-20T01:52:02Z

assigner/baserepo.py


    def _gl_get(self, path, params={}):
        return self.__class__._cls_gl_get(
            self.url_base, path, self.token, params
        )

-    def _gl_post(self, path, payload={}, params={}):
+    def _gl_post(self, path, payload={}, _=None):


We should actually probably be passing params through to _cls_gl_post here...

Yep, done. This is when happens when you fix lint without reading code.

LinuxMercedes · 2018-01-20T02:13:42Z

pylintrc

@@ -143,6 +143,7 @@ disable=print-statement,
        too-many-statements,
        attribute-defined-outside-init,
        redefined-builtin,
+        dangerous-default-value,


I'd like to keep this warning on by default; in general it's correct. In baserepo.py we can do something like #pylint: disable=dangerous-default-value to silence just those warnings.

Done. I agree.

LinuxMercedes · 2018-01-20T02:25:24Z

I've poked this some and it seems to work just fine! I'll probably run it through its paces on 5201 homework 1 before I'm comfortable merging it, though.

@brhoades does this fix any bugs or is it "just" syntax changes? If it's the latter, I will probably just roll this into the next release, whenever that happens.

LinuxMercedes · 2018-01-20T02:37:34Z

CONTRIBUTING.md

@@ -21,6 +21,7 @@ Before merging:
 - Describe the change in `CHANGELOG.md`
 - Make sure `pyflakes assigner` passes with no errors/warnings
 - Check `pylint assigner` and make sure it's not too egregious. (Eventually we'll get the code to a point where there are no messages from `pylint`...)


We should change this to require pylint to pass now that you've done all the work!!!

Made dangerous-default-args a warning again.

brhoades · 2018-01-20T03:36:49Z

There was some bug that pylint claimed to find where BaseRepo.name = "" was overriding StudentRepo.name(), but since the latter is a classmethod this was a nonissue.

LinuxMercedes · 2018-01-23T02:43:00Z

@brhoades Can you clean the commit history up some and then let's merge this!

brhoades · 2018-01-23T03:18:45Z

I'll just squash on merge.

Added travis config file.

c5ecef9

LinuxMercedes requested changes Jan 18, 2018

View reviewed changes

Added pylint to requirements. Fixed travis call.

b68adcb

LinuxMercedes requested changes Jan 18, 2018

View reviewed changes

brhoades added 3 commits January 18, 2018 18:29

Resolved most pylint errors.

27dfa2f

StudentRepo.name -> StudentRepo.build_name

47c32ac

name was being shadowed by an instance var.

Removed FIXME for something that was fixed, created #95 for the other.

1d58edb

brhoades commented Jan 19, 2018

View reviewed changes

brhoades added 3 commits January 18, 2018 18:58

Small changes after a second pass.

2936b44

Updated changelog.

167912a

Updated changelog (again).

5c3fd31

LinuxMercedes reviewed Jan 19, 2018

View reviewed changes

LinuxMercedes added this to the Quality Code milestone Jan 19, 2018

LinuxMercedes reviewed Jan 19, 2018

View reviewed changes

brhoades added 3 commits January 19, 2018 16:47

Restored ={} default params, split changelog.

51f4351

Added pylint ignore for ={}, cleaned up unused params.

ff4477f

Added requirements.txt information to CONTRIBUTING.

4bd3071

LinuxMercedes reviewed Jan 20, 2018

View reviewed changes

Passing params that were unused through in baserepo, updated changelog.

6f07c75

Made dangerous-default-args a warning again.

LinuxMercedes approved these changes Jan 23, 2018

View reviewed changes

brhoades changed the title ~~WIP: Added Travis CI Configuration~~ Added Travis CI Configuration Jan 23, 2018

brhoades merged commit 9f427b3 into master Jan 23, 2018

LinuxMercedes deleted the fb-travis branch February 7, 2018 02:41

LinuxMercedes removed this from the Quality Code milestone Dec 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Travis CI Configuration #94

Added Travis CI Configuration #94

brhoades commented Jan 18, 2018 •

edited

Loading

LinuxMercedes Jan 18, 2018

brhoades Jan 18, 2018

LinuxMercedes Jan 18, 2018

brhoades Jan 19, 2018

LinuxMercedes Jan 19, 2018

brhoades commented Jan 19, 2018

brhoades Jan 19, 2018

LinuxMercedes Jan 19, 2018

brhoades Jan 19, 2018

brhoades Jan 19, 2018

brhoades Jan 19, 2018

brhoades Jan 19, 2018

LinuxMercedes Jan 19, 2018

brhoades Jan 19, 2018

LinuxMercedes Jan 19, 2018

LinuxMercedes Jan 19, 2018

brhoades Jan 20, 2018

LinuxMercedes commented Jan 19, 2018

LinuxMercedes Jan 19, 2018

LinuxMercedes Jan 20, 2018

brhoades Jan 20, 2018

LinuxMercedes Jan 20, 2018

brhoades Jan 20, 2018

LinuxMercedes commented Jan 20, 2018

LinuxMercedes Jan 20, 2018

brhoades Jan 20, 2018

brhoades commented Jan 20, 2018

LinuxMercedes commented Jan 23, 2018

brhoades commented Jan 23, 2018

		@@ -1,5 +1,7 @@
		## Devel

		- Removed lint, removed old baserepo standalone code, and added travis config.

Added Travis CI Configuration #94

Added Travis CI Configuration #94

Conversation

brhoades commented Jan 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brhoades commented Jan 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LinuxMercedes commented Jan 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LinuxMercedes commented Jan 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brhoades commented Jan 20, 2018

LinuxMercedes commented Jan 23, 2018

brhoades commented Jan 23, 2018

brhoades commented Jan 18, 2018 •

edited

Loading