ci: use a named docker instance with proper working dir and env #1011
Conversation
Current coverage is 96.35% (diff: 100%)@@ master #1011 diff @@
==========================================
Files 194 194
Lines 17755 17755
Methods 0 0
Messages 0 0
Branches 1368 1368
==========================================
Hits 17107 17107
Misses 440 440
Partials 208 208
|
sergiusens
changed the title
|
@3v1n0 Thanks. What were we missing without the TERM variable? Can you please report a bug for the problem that you are fixing? |
And export there the host's TERM variable.
3v1n0
force-pushed
the
travis-docker-pwd
branch
from
d31408c
to
c43f1df
Compare
|
@ElOpio bug added.
This will ensure that the docker instance will inherit the TERM variable from the parent terminal, thus it will optionally hide all the unneeded content from logs (if set to TERM=dumb) as per the same reasons that it was introduced in PR: #906. |
3v1n0
force-pushed
the
travis-docker-pwd
branch
from
c43f1df
to
3bd11e8
Compare
| '"apt update -qq && apt install snapcraft -y && cd $(pwd) && ' | ||
| 'snapcraft && snapcraft push *.snap --release edge"'), | ||
| 'docker run -e TERM -v $PWD:$PWD -w $PWD -t snapcore/snapcraft' | ||
| ' sh -c "snapcraft && snapcraft push *.snap --release edge"'), |
There was a problem hiding this comment.
@3v1n0 I think this will also need apt update and apt upgrade, because the snapcore/snapcraft image is not being updated: https://bugs.launchpad.net/snapcraft/+bug/1639112
|
I like -w instead of cd, and I didn't know about it, thanks. However, being a little pita, there are three bugs here:
I'm ok with fixing the three in the same PR, but my problem is that the description of the PR only mentions 1. The bug mentions 2 and 3. And the PR fixes 1 and 2. :) Also, this is not your fault, but you are changing two things in the code, and only one in the tests. That means that we are missing a test. |
3v1n0
changed the title
|
you might want to look at #1125 wrt usage of |
|
Where does this PR stand? |
|
Closing due to lack of activity, feel free to reopen once updated. |
And export there the host's TERM variable.
LP: #1659372