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.

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

config: qualify the name of the version field #309

Merged
merged 1 commit into from
Jan 18, 2016

Conversation

vbatts
Copy link
Member

@vbatts vbatts commented Jan 13, 2016

#110

Signed-off-by: Vincent Batts vbatts@hashbangbash.com

@vbatts
Copy link
Member Author

vbatts commented Jan 13, 2016

Closes #110

@tianon
Copy link
Member

tianon commented Jan 13, 2016

LGTM

1 similar comment
@vishh
Copy link
Contributor

vishh commented Jan 13, 2016

LGTM

@@ -5,7 +5,7 @@ package specs
// bundle is packaged for distribution.
type Spec struct {
// Version is the version of the specification that is supported.
Version string `json:"version"`
Version string `json:"spec_version"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather have a more specific name. Presumably the other versioned things were distinguishing from also have specs (and the current JSON pattern is camelCase, not underscores).


* **`version`** (string, required) must be in [SemVer v2.0.0](http://semver.org/spec/v2.0.0.html) format and specifies the version of the OCF specification with which the container bundle complies. The Open Container spec follows semantic versioning and retains forward and backward compatibility within major versions. For example, if an implementation is compliant with version 1.0.1 of the spec, it is compatible with the complete 1.x series. NOTE that there is no guarantee for forward or backward compatibility for version 0.x.
* **`spec_version`** (string, required) must be in [SemVer v2.0.0](http://semver.org/spec/v2.0.0.html) format and specifies the version of the OpenContainer specification with which the bundle complies.
Copy link
Contributor

Choose a reason for hiding this comment

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

should be specVersion

Copy link
Member Author

Choose a reason for hiding this comment

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

i already pushed a change for this. PTAL

On Thu, Jan 14, 2016 at 5:16 PM Mrunal Patel notifications@github.com
wrote:

In config.md
#309 (comment):

-* version (string, required) must be in SemVer v2.0.0 format and specifies the version of the OCF specification with which the container bundle complies. The Open Container spec follows semantic versioning and retains forward and backward compatibility within major versions. For example, if an implementation is compliant with version 1.0.1 of the spec, it is compatible with the complete 1.x series. NOTE that there is no guarantee for forward or backward compatibility for version 0.x.
+* spec_version (string, required) must be in SemVer v2.0.0 format and specifies the version of the OpenContainer specification with which the bundle complies.

should be specVersion


Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/309/files#r49793850.

Copy link
Contributor

Choose a reason for hiding this comment

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

On Thu, Jan 14, 2016 at 02:37:10PM -0800, Vincent Batts wrote:

+* spec_version (string, required) must be in SemVer v2.0.0 format and specifies the version of the OpenContainer specification with which the bundle complies.

i already pushed a change for this. PTAL

4df6c3b (the current tip) has specVersion in config.go but
spec_version in config.md (twice). Are we sure we don't want a more
specific name [1,2]?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to more specific name.

On Fri, Jan 15, 2016 at 12:25 AM, W. Trevor King notifications@github.com
wrote:

In config.md
#309 (comment):

-* version (string, required) must be in SemVer v2.0.0 format and specifies the version of the OCF specification with which the container bundle complies. The Open Container spec follows semantic versioning and retains forward and backward compatibility within major versions. For example, if an implementation is compliant with version 1.0.1 of the spec, it is compatible with the complete 1.x series. NOTE that there is no guarantee for forward or backward compatibility for version 0.x.
+* spec_version (string, required) must be in SemVer v2.0.0 format and specifies the version of the OpenContainer specification with which the bundle complies.

On Thu, Jan 14, 2016 at 02:37:10PM -0800, Vincent Batts wrote: > +*
spec_version (string, required) must be in SemVer v2.0.0 format and specifies the version of
the OpenContainer specification with which the bundle complies. i already
pushed a change for this. PTAL
4df6c3b
4df6c3b
(the current tip) has specVersion in config.go but spec_version in
config.md (twice). Are we sure we don't want a more specific name [1,2]?
[1]: #309 (comment)
#309 (comment)
[2]: #110 (comment)
#110 (comment)


Reply to this email directly or view it on GitHub
https://github.com/opencontainers/specs/pull/309/files#r49801534.

Copy link
Contributor

Choose a reason for hiding this comment

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

ociVersion

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to ociVersion

Sent from my iPhone

On Jan 15, 2016, at 6:28 AM, Doug Davis notifications@github.com wrote:

In config.md:

-* version (string, required) must be in SemVer v2.0.0 format and specifies the version of the OCF specification with which the container bundle complies. The Open Container spec follows semantic versioning and retains forward and backward compatibility within major versions. For example, if an implementation is compliant with version 1.0.1 of the spec, it is compatible with the complete 1.x series. NOTE that there is no guarantee for forward or backward compatibility for version 0.x.
+* spec_version (string, required) must be in SemVer v2.0.0 format and specifies the version of the OpenContainer specification with which the bundle complies.
ociVersion


Reply to this email directly or view it on GitHub.

opencontainers#110

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
@vbatts
Copy link
Member Author

vbatts commented Jan 15, 2016

updated to ociVersion. PTAL

@duglin
Copy link
Contributor

duglin commented Jan 15, 2016

+1

@tianon
Copy link
Member

tianon commented Jan 15, 2016

re-LGTM

@wking
Copy link
Contributor

wking commented Jan 15, 2016

ociVersion is better, but we may end up with multiple OCI specs.
We're calling this spec “Open Container Specifications” 1, so why
not “ocsVersion”?

@duglin
Copy link
Contributor

duglin commented Jan 15, 2016

Not to take us down too much of a tangent, but do we have one spec or multiple in this project? If we have multiple then "ocs" doesn't help since having "s=spec" when there are multiple is kind of ambiguous. Perhaps "Open Container Runtime (ocr)" for the core spec we've been talking about. But if we view this all as one big spec (which might be nice to avoid each one having a different version number), then just "oci" would work.

@wking
Copy link
Contributor

wking commented Jan 15, 2016

On Fri, Jan 15, 2016 at 10:35:34AM -0800, Doug Davis wrote:

Not to take us down too much of a tangent, but do we have one spec
or multiple in this project?

I would love to have this question answered, since I'm frequently
suggesting we handle orthogonal issues in separate OCI specs
(e.g. 1). But to stay on topic here, maybe we should move this
discussion to the list?

 Subject: Re: appc + oci harmonization progress
 Date: Thu, 13 Aug 2015 11:29:08 -0700
 Message-ID: <20150813182908.GS412@odin.tremily.us>

@hqhq
Copy link
Contributor

hqhq commented Jan 18, 2016

We should leave the discussion about the needs of multiple OCI spec to the list.
LGTM

hqhq added a commit that referenced this pull request Jan 18, 2016
config: qualify the name of the version field
@hqhq hqhq merged commit ec7ca91 into opencontainers:master Jan 18, 2016
@vbatts vbatts deleted the version_name branch April 12, 2016 03:51
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jun 4, 2016
This reverts commit 0f25f18, opencontainers#253.
Now that we're on to 1.0, we don't need to talk about 0.x.  And the
lack of 0.x backwards compatability is covered by SemVer 2.0 section 4
[1]:

  Major version zero (0.y.z) is for initial development.  Anything may
  change at any time.  The public API should not be considered stable.

so removing the echo from our spec doesn't actually change anything.

The conflict is due to 4e63ee0 (config: qualify the name of the
version field, 2016-01-13, opencontainers#309), and only impacted the context and
line-wrapping around the sentence I'm removing.

Conflicts:
	config.md

[1]: http://semver.org/spec/v2.0.0.html

Signed-off-by: W. Trevor King <wking@tremily.us>
Mashimiao pushed a commit to Mashimiao/specs that referenced this pull request Aug 19, 2016
This reverts commit 0f25f18, opencontainers#253.
Now that we're on to 1.0, we don't need to talk about 0.x.  And the
lack of 0.x backwards compatability is covered by SemVer 2.0 section 4
[1]:

  Major version zero (0.y.z) is for initial development.  Anything may
  change at any time.  The public API should not be considered stable.

so removing the echo from our spec doesn't actually change anything.

The conflict is due to 4e63ee0 (config: qualify the name of the
version field, 2016-01-13, opencontainers#309), and only impacted the context and
line-wrapping around the sentence I'm removing.

Conflicts:
	config.md

[1]: http://semver.org/spec/v2.0.0.html

Signed-off-by: W. Trevor King <wking@tremily.us>
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.

8 participants