Skip to content

Conversation

@wking
Copy link
Contributor

@wking wking commented Nov 22, 2016

Cherry-picking #260 onto master.

@wking wking force-pushed the runtimetest-coverage-caveat-2 branch from 580fd0f to afbe433 Compare November 22, 2016 18:19
@liangchenye
Copy link
Member

liangchenye commented Nov 23, 2016

LGTM

Approved with PullApprove

@Mashimiao
Copy link

I think there is no need to add test files in urfave/cli.

@wking
Copy link
Contributor Author

wking commented Nov 23, 2016

On Tue, Nov 22, 2016 at 05:50:45PM -0800, Ma Shimiao wrote:

I think there is no need to add test files in urfave/cli.

I'm just running ‘godep update github.com/urfave/cli’ (as quoted in
the commit message). If I should be doing something else instead,
just tell me what to paste in ;).

@Mashimiao
Copy link

I also tried to update urfave/cli and no test files were added. Maybe you need to update you godep version

This landed via [1] which was released with v1.19.0.  Bump to the most
recent release to get the .1 bugfixes too [2].  Generated with:

  $ godep update github.com/urfave/cli
  godep: WARNING: Godep workspaces (./Godeps/_workspace) are deprecated and support for them will be removed when go1.8 is released.
  godep: WARNING: Go version (go1.7) & $GO15VENDOREXPERIMENT= wants to enable the vendor experiment, but disabling because a Godep workspace (Godeps/_workspace) exists
  $ git add -A Godeps

[1]: urfave/cli#543
[2]: https://github.com/urfave/cli/blob/v1.19.1/CHANGELOG.md#1191---2016-11-21

Signed-off-by: W. Trevor King <[email protected]>
As I recommended earlier in the context of checking for all configured
devices [1].  An example would be:

  {
    ...
    "linux": {
      "devices": [
        {
          "path": "/dev/fuse",
          ...
        }
      ],
    },
    "hooks": {
      "prestart": [
        {
          "path": "/bin/rm",
          "args": ["rm", "/dev/fuse"]
        }
      }
    }
  }

where the resulting container (when created by a conformant runtime)
would not have the /dev/fuse entry runtimetest's linux.devices check
is looking for.

[1]: opencontainers#211 (comment)

Signed-off-by: W. Trevor King <[email protected]>
@wking wking force-pushed the runtimetest-coverage-caveat-2 branch from afbe433 to 4057aaf Compare November 23, 2016 07:24
@wking
Copy link
Contributor Author

wking commented Nov 23, 2016

On Tue, Nov 22, 2016 at 11:18:42PM -0800, Ma Shimiao wrote:

Maybe you need to update you godep version

That was the problem, thanks :). urfave/cli tests dropped with
afbe4334057aaf.

With godep v75 the update printed two warnings:

godep: WARNING: Godep workspaces (./Godeps/_workspace) are deprecated and support for them will be removed when go1.8 is released.
godep: WARNING: Go version (go1.7) & $GO15VENDOREXPERIMENT= wants to enable the vendor experiment, but disabling because a Godep workspace (Godeps/_workspace) exists

I'm not familiar enough with Godep to know what (if anything) we're
supposed to do to migrate to whatever the new approaches are.

@Mashimiao
Copy link

About godeps and workspace changes, I have create RP #231 to solve this problem.
In my opinion, all problems of godep we can solve in #231
But if you want to update urfave/cli in this PR, I also don't think this is a problem.

@Mashimiao
Copy link

Mashimiao commented Nov 23, 2016

LGTM

Approved with PullApprove

1 similar comment
@mrunalp
Copy link
Contributor

mrunalp commented Nov 23, 2016

LGTM

Approved with PullApprove

@mrunalp mrunalp merged commit 893d9e3 into opencontainers:master Nov 23, 2016
@wking wking deleted the runtimetest-coverage-caveat-2 branch January 21, 2017 15:25
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.

4 participants