Structuring APIs

[hhsecond]

A massive PR includes the API structuring, test cases, CI setup

What it does not contain

• An exhaustive test suite
• mypy check

The APIs might need more changes (one I could think off is the have dictionary style access). The primary motive I had while designing the APIs is making user relying on git checkout as much as possible. But I could see why a git + stockroom workflow should be an option but should not be the only workflow possible especially since stockroom allows model storage without user spending time figuring out how to store models. The next release will be a more organized with more API planning analyzing user stories trying out with this release

[rlizzo]

I think alot of the comments below aren't so much requests for changes, or large issues, but just general considerations about design and intentions.

There's a lot that's left unclear here. The API really needs to be documented somwhere. If it's not in documentation, then atleast in tests. It's hard to follow the intended code flow without some sort of reference as to where data comes in and out from, and what is exposed directly to the user and what is managed by stockroom internally.

As a proof of concept, its good enough for now, but these are just a few things which need consideration before taking this to a "ready for alpha" stage.

[rlizzo]

@hhsecond why did you decide not to go with the src/stockroom layout?

[rlizzo]

Quite a large PR! This is much more polished, and shows quite a bit of promise for stockroom in the future - in general, nice work!

There are a few issues i've identified which need to be fixed, some which you can use your discression on, and some that are just more for my own understanding.

I haven't tested the code myself yet, so more may follow in the next day or two.

Thanks for the hard work Sherin! looking forward to @lantiga's review!

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@391e143). Click here to learn what that means.
The diff coverage is 92.68%.

@@            Coverage Diff            @@
##             master       #1   +/-   ##
=========================================
  Coverage          ?   90.94%           
=========================================
  Files             ?       15           
  Lines             ?      574           
  Branches          ?       42           
=========================================
  Hits              ?      522           
  Misses            ?       41           
  Partials          ?       11

Impacted Files	Coverage Δ
stockroom/cli.py	0% <0%> (ø)
tests/conftest.py	100% <100%> (ø)
stockroom/parser.py	100% <100%> (ø)
tests/test_data.py	100% <100%> (ø)
tests/test_tag.py	100% <100%> (ø)
tests/test_model.py	100% <100%> (ø)
stockroom/storages/__init__.py	100% <100%> (ø)
stockroom/storages/data.py	100% <100%> (ø)
tests/test_repository.py	100% <100%> (ø)
stockroom/storages/tag.py	85.18% <85.18%> (ø)
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 391e143...4c3e15e. Read the comment docs.

[lgtm-com]

This pull request introduces 1 alert when merging a07b749 into 391e143 - view on LGTM.com

new alerts:

• 1 for Unused import

[lgtm-com]

This pull request fixes 4 alerts when merging 4406681 into 391e143 - view on LGTM.com

fixed alerts:

• 4 for Module imports itself

[lgtm-com]

This pull request fixes 4 alerts when merging 50b5e7c into 391e143 - view on LGTM.com

fixed alerts:

• 4 for Module imports itself

[lgtm-com]

This pull request fixes 4 alerts when merging 24b6808 into 391e143 - view on LGTM.com

fixed alerts:

• 4 for Module imports itself

[lgtm-com]

This pull request fixes 4 alerts when merging 2d93209 into 391e143 - view on LGTM.com

fixed alerts:

• 4 for Module imports itself

[lgtm-com]

This pull request fixes 4 alerts when merging 6a95aa2 into 391e143 - view on LGTM.com

fixed alerts:

• 4 for Module imports itself

[lgtm-com]

This pull request fixes 4 alerts when merging 87f994b into 391e143 - view on LGTM.com

fixed alerts:

• 4 for Module imports itself

[lgtm-com]

This pull request fixes 4 alerts when merging 4c3e15e into 391e143 - view on LGTM.com

fixed alerts:

• 4 for Module imports itself