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.

Sign up for GitHub

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

Jump to bottom

docker: add the architecture #1994

Merged
merged 1 commit into from
Mar 15, 2018
Merged

docker: add the architecture #1994

merged 1 commit into from
Mar 15, 2018

Conversation

sergiusens
Copy link
Collaborator

@sergiusens sergiusens commented Mar 8, 2018 •
edited

Setup the correct SNAP_ARCH for our docker images, also blow up in a more
obvious way if it is missing.

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 unit?

@sergiusens sergiusens self-assigned this Mar 8, 2018
@sergiusens sergiusens added the bug Actual bad behavior that don't fall into maintenance or documentation label Mar 8, 2018
@sergiusens sergiusens added this to the 2.40 milestone Mar 8, 2018
kenvandine
kenvandine approved these changes Mar 8, 2018
View reviewed changes
Copy link
Contributor

@kenvandine kenvandine left a comment

Choose a reason for hiding this comment

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

Looks good!

kyrofa
kyrofa reviewed Mar 8, 2018
View reviewed changes
docker/bin/snapcraft-wrapper Outdated
@@ -1,3 +1,7 @@
#!/bin/sh
export SNAP_VERSION="$(awk '/^version:/{print $2}' '/snap/snapcraft/current/meta/snap.yaml')"
export SNAP=/snap/snapcraft/current
export SNAP_NAME=export SNAP_VERSION="$(awk '/^name:/{print $2}' $SNAP/meta/snap.yaml)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Too many exports, here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch

kyrofa
kyrofa approved these changes Mar 13, 2018
View reviewed changes
Copy link
Contributor

@kyrofa kyrofa left a comment •
edited

Choose a reason for hiding this comment

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

Looks good now, thanks. Tests don't look so hot, though. Your adding of the armhf line may have thrown the other patch lines off.

@sergiusens
docker: export SNAP_ARCH into the environment
87ff30c 
Setup the correct SNAP_ARCH for our docker images.

Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
@kyrofa
Copy link
Contributor

kyrofa commented Mar 14, 2018

(still +1 by the way)

@sergiusens sergiusens merged commit 54d7494 into canonical:master Mar 15, 2018
@sergiusens sergiusens deleted the docker-fix branch March 15, 2018 10:17
@sergiusens sergiusens added this to Bugs & Tech debt in 18.04 Mar 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kenvandine kenvandine kenvandine approved these changes

@kyrofa kyrofa kyrofa approved these changes

Assignees

@sergiusens sergiusens

Labels
bug Actual bad behavior that don't fall into maintenance or documentation
Projects
18.04
Bugs & Tech debt
Milestone
2.40
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sergiusens @kyrofa @kenvandine