Skip to content
This repository has been archived by the owner on Feb 22, 2020. It is now read-only.

Improve various code styles #309

Merged
merged 23 commits into from
Mar 1, 2018
Merged

Improve various code styles #309

merged 23 commits into from
Mar 1, 2018

Conversation

lamby
Copy link
Contributor

@lamby lamby commented Feb 27, 2018

This PR includes a large number of code refactoring within the backend (Django) part of the application.

@lamby lamby force-pushed the lamby/wip/code-style branch 2 times, most recently from 67b8406 to 5ee05d3 Compare February 27, 2018 15:59
@lamby
Copy link
Contributor Author

lamby commented Feb 27, 2018

@isms @reubano Any ideas on this testsuite failure? Can't seem to work out what is actually really broken, let alone how I could have done it...

@reubano
Copy link
Contributor

reubano commented Feb 27, 2018

Possibly related to #305 (comment)

@isms
Copy link
Contributor

isms commented Feb 27, 2018

@reubano Can you dig in and find out what the source is?

@lamby I say we let that tests fail for a while while Reuben looks into it so you're not blocked :)

@lamby
Copy link
Contributor Author

lamby commented Feb 28, 2018

As in, I can't quite understand the error message!

@reubano
Copy link
Contributor

reubano commented Feb 28, 2018

@reubano Can you dig in and find out what the source is?

@isms @lamby can either of you verify that bug?

@lamby
Copy link
Contributor Author

lamby commented Feb 28, 2018

@isms I'm not blocked, more that I wouldn't want to be introducing any new issue. Indeed, this PR can be merged (renaming to match)

@lamby lamby changed the title Improve various code styles [WIP] Improve various code styles Feb 28, 2018
@@ -1,4 +1,4 @@
from collections import OrderedDict
import collections
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you prefer to import the whole module here while you changed in 5302cfe to directly import methods from a module?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in general I prefer to import the modules that Python ships with ("batteries included") as top-level modules. However, I am not 100% dogmatic on this (!) and make some exceptions if that's going to make the code unspeakably ugly. For example,, imagine trying to parse:

os.path.join(os.path.join(os.path.join(os.path.ditpath(os.path.join(os.path.join(os.path.join(....)))

Make sense? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thank you :)

@lamby
Copy link
Contributor Author

lamby commented Mar 1, 2018

Any luck all? Would love to get this merged! :)

@isms isms merged commit fda6f6b into master Mar 1, 2018
@lamby lamby deleted the lamby/wip/code-style branch March 2, 2018 08:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants