Skip to content
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

store: support registering to a specific store #2479

Merged
merged 5 commits into from
Mar 14, 2019

Conversation

sergiusens
Copy link
Collaborator

Add a --store-id option to the register command.

Signed-off-by: Sergio Schvezov sergio.schvezov@canonical.com

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • If this is a bugfix. Have you checked that there is a bug report open for the issue you are trying to fix on bug reports?
  • If this is a new feature. Have you discussed the design on the forum?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh tests/unit?

Copy link
Contributor

@sparkiegeek sparkiegeek left a comment

Choose a reason for hiding this comment

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

Think we should call this store instead of store-id - there are unfortunately many ways to refer to a store, and the API docs call it store

snapcraft/cli/store.py Outdated Show resolved Hide resolved
snapcraft/cli/store.py Outdated Show resolved Hide resolved
snapcraft/cli/store.py Outdated Show resolved Hide resolved
snapcraft/storeapi/_sca_client.py Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Feb 25, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2479   +/-   ##
=========================================
  Coverage          ?   89.94%           
=========================================
  Files             ?      198           
  Lines             ?    13367           
  Branches          ?     2024           
=========================================
  Hits              ?    12023           
  Misses            ?      921           
  Partials          ?      423
Impacted Files Coverage Δ
snapcraft/cli/store.py 88.88% <100%> (ø)
snapcraft/storeapi/_sca_client.py 86.81% <100%> (ø)
snapcraft/storeapi/_store_client.py 83.16% <100%> (ø)
snapcraft/_store.py 94.26% <100%> (ø)

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 90b875c...1ebf63f. Read the comment docs.

@sergiusens sergiusens force-pushed the register-on-store-brand branch from 3e0ec18 to bd3010c Compare March 13, 2019 16:20
Copy link
Contributor

@cmatsuoka cmatsuoka left a comment

Choose a reason for hiding this comment

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

I don't have full context on store IDs but the overall implementation looks sane.

sergiusens and others added 5 commits March 14, 2019 11:21
Add a --store-id option to the register command.

Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
Co-Authored-By: sergiusens <sergio.schvezov@canonical.com>
Revert mass store_id rename to only the user facing bits as
store is an object used in decorators and contexts that take precedence

Add an integration test for a valid registration and an invalid one.

Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
@sergiusens sergiusens force-pushed the register-on-store-brand branch from 7e7af7e to 1ebf63f Compare March 14, 2019 14:22
@sergiusens sergiusens merged commit 636d6be into canonical:master Mar 14, 2019
@sergiusens sergiusens deleted the register-on-store-brand branch March 14, 2019 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants