Skip to content

Snap packaging#232

Merged
blakerouse merged 7 commits intocanonical:masterfrom
blakerouse:snap-packaging
Oct 3, 2019
Merged

Snap packaging#232
blakerouse merged 7 commits intocanonical:masterfrom
blakerouse:snap-packaging

Conversation

@blakerouse
Copy link
Copy Markdown
Contributor

@blakerouse blakerouse commented Sep 24, 2019

Adds snap/snapcraft.yaml and improves the help menu to use the snap instance name. Add maas-cli content interface so "maas" snap can connect to this snap to get the latest CLI.

@blakerouse blakerouse force-pushed the snap-packaging branch 2 times, most recently from a9ef201 to 4d611b4 Compare September 24, 2019 17:22
@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 24, 2019

Codecov Report

Merging #232 into master will decrease coverage by 0.05%.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #232      +/-   ##
==========================================
- Coverage   67.13%   67.08%   -0.06%     
==========================================
  Files          70       70              
  Lines        5715     5723       +8     
  Branches      985      987       +2     
==========================================
+ Hits         3837     3839       +2     
- Misses       1668     1674       +6     
  Partials      210      210
Impacted Files Coverage Δ
maas/client/flesh/__init__.py 38.63% <25%> (-0.43%) ⬇️

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 e33e1af...63a674c. Read the comment docs.

@blakerouse blakerouse force-pushed the snap-packaging branch 2 times, most recently from 23a36b1 to b91b42d Compare September 24, 2019 17:40
Copy link
Copy Markdown
Contributor

@albertodonato albertodonato left a comment

Choose a reason for hiding this comment

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

A few comments/questions

Comment thread maas/client/flesh/__init__.py Outdated
if os.environ.get("SNAP_INSTANCE_NAME"):
return os.environ.get("SNAP_INSTANCE_NAME")
if os.environ.get("SNAP_NAME"):
return os.environ.get("SNAP_NAME")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you shouldn't need this, SNAP_INSTANCE_NAME is always there for the snap

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay will remove it, didn't know if that would always be the case, if you had a older snapd.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that got introduced way before content interface :)

Comment thread snap/snapcraft.yaml
interface: content
content: maas-cli
read:
- $SNAP/lib
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can't we just expose the path of the library?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We need all the dependencies not just the libmaas pieces, so I thought it best to expose the whole lib section. Only thing under there is lib/python3.6/site-packages anyway.

Copy link
Copy Markdown
Contributor

@albertodonato albertodonato left a comment

Choose a reason for hiding this comment

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

+1

@blakerouse blakerouse merged commit 647065b into canonical:master Oct 3, 2019
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.

3 participants