Skip to content
This repository has been archived by the owner on Sep 23, 2024. It is now read-only.

Refactor patchwork.py #38

Open
spbnick opened this issue Apr 26, 2018 · 2 comments
Open

Refactor patchwork.py #38

spbnick opened this issue Apr 26, 2018 · 2 comments

Comments

@spbnick
Copy link
Contributor

spbnick commented Apr 26, 2018

The sktm/patchwork.py needs refactoring as it's hard to understand, use and maintain. Some ideas include:

  • Extract common interface into an abstract class to be the base for both Patchwork interfaces.
  • Make methods and variables not used from outside private
  • Create a type/class for data both implementations need about a patch, make the implementations provide functions for fetching those and implement the rest of the handling in the base, abstract class.
  • Separate classes into their own files. This way it's easier to look at implementations separately. Right now, when searching for an implementation of a function, you often don't know which class you're looking at, which makes it difficult to understand what's going on.
  • Simplify and narrow down the public interfaces
  • Include Refactor patchwork.py to not need trivial comments #35.

More ideas are welcome!

@spbnick
Copy link
Contributor Author

spbnick commented Apr 26, 2018

Also, rename the classes to make more sense. E.g. Patchwork for the base class, PatchworkXMLRPC for XML RPC interface, and PatchworkREST for the REST API.

@veruu
Copy link
Contributor

veruu commented Jul 4, 2018

Note to self: get_patch_by_id for PW2 raises an exception when it can't get the data needed, while PW1just logs that it couldn't get the data and returns None. We should throw the exception in both cases, and this would also allow us to remove the checks for no patch returned elsewhere.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants