Skip to content

Conversation

@wking
Copy link
Contributor

@wking wking commented Jun 18, 2016

I'd added some omitempties in 5c2193f (specs-go/config: Make Linux and Solaris omitempty, 2016-05-06, #431), but it turns out to not have the intended effect unless the field is also a pointer type (even after I shifted the omitempty from the platform tag to the json tag ;). Before this commit:

$ ./ocitools generate --template <(echo '{}')
$ jq . config.json
{
  "ociVersion": "1.0.0-rc1-dev",
  "platform": {
    "os": "linux",
    "arch": "amd64"
  },
  "process": {
    "user": {
      "uid": 0,
      "gid": 0
    },
    "args": [],
    "cwd": "/"
  },
  "root": {
    "path": "rootfs"
  },
  "hooks": {},
  "linux": {
    "cgroupsPath": ""
  },
  "solaris": {
    "cappedCPU": {},
    "cappedMemory": {}
  }
}

And after this commit:

$ ./ocitools generate --template <(echo '{}')
$ jq . config.json
{
  "ociVersion": "1.0.0-rc1-dev",
  "platform": {
    "os": "linux",
    "arch": "amd64"
  },
  "process": {
    "user": {
      "uid": 0,
      "gid": 0
    },
    "args": [],
    "cwd": "/"
  },
  "root": {
    "path": "rootfs"
  },
  "hooks": {},
}

The remaining useless properties are addressed by other in-flight pull requests:

So I've left them alone here.

@wking wking force-pushed the optional-linux-solaris branch from c81826f to 1f605d1 Compare June 18, 2016 05:13
I'd added some omitempties in 5c2193f (specs-go/config: Make Linux
and Solaris omitempty, 2016-05-06, opencontainers#431), but it turns out to not have
the intended effect unless the field is also a pointer type (even
after I shifted the 'omitempty' from the platform tag to the json
tag).  Before this commit:

  $ ./ocitools generate --template <(echo '{}')
  $ jq . config.json
  {
    "ociVersion": "1.0.0-rc1-dev",
    "platform": {
      "os": "linux",
      "arch": "amd64"
    },
    "process": {
      "user": {
        "uid": 0,
        "gid": 0
      },
      "args": [],
      "cwd": "/"
    },
    "root": {
      "path": "rootfs"
    },
    "hooks": {},
    "linux": {
      "cgroupsPath": ""
    },
    "solaris": {
      "cappedCPU": {},
      "cappedMemory": {}
    }
  }

And after this commit:

  $ ./ocitools generate --template <(echo '{}')
  $ jq . config.json
  {
    "ociVersion": "1.0.0-rc1-dev",
    "platform": {
      "os": "linux",
      "arch": "amd64"
    },
    "process": {
      "user": {
        "uid": 0,
        "gid": 0
      },
      "args": [],
      "cwd": "/"
    },
    "root": {
      "path": "rootfs"
    },
    "hooks": {},
  }

The remaining useless properties are addressed by other in-flight pull
requests:

* 5ca74df (config: Make 'process.args' optional, 2016-06-04, opencontainers#489)
* ad33f9c (config: Explicitly list 'hooks' as optional, 2016-05-06,
  opencontainers#427)

So I've left them alone here.

Signed-off-by: W. Trevor King <[email protected]>
@dqminh
Copy link
Contributor

dqminh commented Jun 22, 2016

LGTM

Approved with PullApprove

@crosbymichael
Copy link
Member

crosbymichael commented Jun 24, 2016

LGTM

Approved with PullApprove

@crosbymichael crosbymichael merged commit b45aa77 into opencontainers:master Jun 24, 2016
@wking wking deleted the optional-linux-solaris branch August 18, 2016 04:54
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Sep 21, 2016
Otherwise we it will never be omitted when empty.  This is the same
case as 6323157 (specs-go/config: Make Linux and Solaris omitempty
(again), 2016-06-17, opencontainers#502).

Signed-off-by: W. Trevor King <[email protected]>
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