Skip to content

Conversation

@Mashimiao
Copy link

Signed-off-by: Ma Shimiao [email protected]

@wking
Copy link
Contributor

wking commented Oct 21, 2016

6afe0d8 looks fine to me. But pull requests for every setting in runtime-spec seems like a lot of work when:

$ echo '{}' |
>   jq '.linux.maskedPaths = ["/proc/kcore", "/proc/latency_stats"]'
{
  "linux": {
    "maskedPaths": [
      "/proc/kcore",
      "/proc/latency_stats"
    ]
  }
}
$ echo '{}' |
>   jq '.linux.maskedPaths = ["/proc/kcore", "/proc/latency_stats"]' |
>   jq '.linux.maskedPaths |= . + ["/proc/timer_stats"]'
{
  "linux": {
    "maskedPaths": [
      "/proc/kcore",
      "/proc/latency_stats",
      "/proc/timer_stats"
    ]
  }
}

and:

g.Spec.Linux.MaskedPaths = append(g.Spec.Linux.MaskedPaths, "/proc/timer_stats")

are so straightforward. Are we sure we don't want to revisit making Generator.Spec public? Then we could have the library just provide helpers for situations that are trickier than an append(…) call.

@mrunalp
Copy link
Contributor

mrunalp commented Oct 23, 2016

@wking I think that we can make the field public but let's get this one in for now. LGTM.

@mrunalp mrunalp merged commit a773a29 into opencontainers:master Oct 23, 2016
wking added a commit to wking/ocitools-v2 that referenced this pull request Nov 6, 2016
This makes the attribute public, since quite a few config
manipulations are easier using Go's types than they are via
getter/setter/mutator methods.  This also means that we can drop
methods that are more awkward than direct access (although we'll want
to keep methods that are more convenient than direct access).  I
haven't done any method-dropping in this commit though, aside from the
getter/setter for the config itself.  I'd called for this back when we
started adding these methods [1], and Mrunal was sounding more
positive about the public-attribute approach a few weeks ago [2].

I've also renamed this from "spec" to "config", because it is a
config.  I'm not sure why runtime-spec decided to call the main
config.go type 'Spec' [3], but I don't think we should repeat that
idiosyncrasy.

[1]: opencontainers#137 (comment)
[2]: opencontainers#253 (comment)
[3]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc2/specs-go/config.go#L6

Signed-off-by: W. Trevor King <[email protected]>
@Mashimiao Mashimiao deleted the generate-add-masked-readonly-options branch November 14, 2016 09:31
wking added a commit to wking/ocitools-v2 that referenced this pull request Nov 22, 2016
This makes the attribute public, since quite a few config
manipulations are easier using Go's types than they are via
getter/setter/mutator methods.  This also means that we can drop
methods that are more awkward than direct access (although we'll want
to keep methods that are more convenient than direct access).  I
haven't done any method-dropping in this commit though, aside from the
getter/setter for the config itself.  I'd called for this back when we
started adding these methods [1], and Mrunal was sounding more
positive about the public-attribute approach a few weeks ago [2].

I've also renamed this from "spec" to "config", because it is a
config.  I'm not sure why runtime-spec decided to call the main
config.go type 'Spec' [3], but I don't think we should repeat that
idiosyncrasy.

[1]: opencontainers#137 (comment)
[2]: opencontainers#253 (comment)
[3]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc2/specs-go/config.go#L6

Signed-off-by: W. Trevor King <[email protected]>
wking added a commit to wking/ocitools-v2 that referenced this pull request Nov 23, 2016
This makes the attribute public, since quite a few config
manipulations are easier using Go's types than they are via
getter/setter/mutator methods.  This also means that we can drop
methods that are more awkward than direct access (although we'll want
to keep methods that are more convenient than direct access).  I
haven't done any method-dropping in this commit though, aside from the
getter/setter for the config itself.  I'd called for this back when we
started adding these methods [1], and Mrunal was sounding more
positive about the public-attribute approach a few weeks ago [2].

I've also renamed this from "spec" to "config", because it is a
config.  I'm not sure why runtime-spec decided to call the main
config.go type 'Spec' [3], but I don't think we should repeat that
idiosyncrasy.

[1]: opencontainers#137 (comment)
[2]: opencontainers#253 (comment)
[3]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc2/specs-go/config.go#L6

Signed-off-by: W. Trevor King <[email protected]>
wking added a commit to wking/ocitools-v2 that referenced this pull request Nov 23, 2016
This makes the attribute public, since quite a few config
manipulations are easier using Go's types than they are via
getter/setter/mutator methods.  This also means that we can drop
methods that are more awkward than direct access (although we'll want
to keep methods that are more convenient than direct access).  I
haven't done any method-dropping in this commit though, aside from the
getter/setter for the config itself.  I'd called for this back when we
started adding these methods [1], and Mrunal was sounding more
positive about the public-attribute approach a few weeks ago [2].

I've also renamed this from "spec" to "config", because it is a
config.  I'm not sure why runtime-spec decided to call the main
config.go type 'Spec' [3], but I don't think we should repeat that
idiosyncrasy.

[1]: opencontainers#137 (comment)
[2]: opencontainers#253 (comment)
[3]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc2/specs-go/config.go#L6

Signed-off-by: W. Trevor King <[email protected]>
wking added a commit to wking/ocitools-v2 that referenced this pull request Feb 14, 2017
This makes the attribute public, since quite a few config
manipulations are easier using Go's types than they are via
getter/setter/mutator methods.  This also means that we can drop
methods that are more awkward than direct access (although we'll want
to keep methods that are more convenient than direct access).  I
haven't done any method-dropping in this commit though, aside from the
getter/setter for the config itself.  I'd called for this back when we
started adding these methods [1], and Mrunal was sounding more
positive about the public-attribute approach a few weeks ago [2].

I've also renamed this from "spec" to "config", because it is a
config.  I'm not sure why runtime-spec decided to call the main
config.go type 'Spec' [3], but I don't think we should repeat that
idiosyncrasy.

[1]: opencontainers#137 (comment)
[2]: opencontainers#253 (comment)
[3]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc2/specs-go/config.go#L6

Signed-off-by: W. Trevor King <[email protected]>
wking added a commit to wking/ocitools-v2 that referenced this pull request Feb 14, 2017
This makes the attribute public, since quite a few config
manipulations are easier using Go's types than they are via
getter/setter/mutator methods.  This also means that we can drop
methods that are more awkward than direct access (although we'll want
to keep methods that are more convenient than direct access).  I
haven't done any method-dropping in this commit though, aside from the
getter/setter for the config itself.  I'd called for this back when we
started adding these methods [1], and Mrunal was sounding more
positive about the public-attribute approach a few weeks ago [2].

I've also renamed this from "spec" to "config", because it is a
config.  I'm not sure why runtime-spec decided to call the main
config.go type 'Spec' [3], but I don't think we should repeat that
idiosyncrasy.

[1]: opencontainers#137 (comment)
[2]: opencontainers#253 (comment)
[3]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc2/specs-go/config.go#L6

Signed-off-by: W. Trevor King <[email protected]>
wking added a commit to wking/ocitools-v2 that referenced this pull request Apr 12, 2018
This makes the attribute public, since quite a few config
manipulations are easier using Go's types than they are via
getter/setter/mutator methods.  This also means that we can drop
methods that are more awkward than direct access (although we'll want
to keep methods that are more convenient than direct access).  I
haven't done any method-dropping in this commit though, although I
have deprecated a few of the more redundant methods.  I'd called for
this back when we started adding these methods [1], and Mrunal was
sounding more positive about the public-attribute approach a few weeks
ago [2].

I've also renamed this from "spec" to "config", because it is a
config.  I'm not sure why runtime-spec decided to call the main
config.go type 'Spec' [3], but I don't think we should repeat that
idiosyncrasy.

[1]: opencontainers#137 (comment)
[2]: opencontainers#253 (comment)
[3]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc2/specs-go/config.go#L6

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