/charm

Rename SupportedSeries and add json serialisation #158

Merged
merged 2 commits into from Sep 30, 2015

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@wallyworld @rogpeppe @jujubot
@wallyworld
Owner

wallyworld commented Sep 29, 2015

The recent rename of Series to SupportedSeries was rejected, so we rename back.
meta.go
+ OldRevision int `bson:"oldrevision,omitempty"` // Obsolete
+ Categories []string `bson:"categories,omitempty" json:"categories,omitempty"`
+ Tags []string `bson:"tags,omitempty" json:"tag,omitempty"`
+ Series []string `bson:"series,omitempty" json:"supported-series,omitempty"`
@rogpeppe

rogpeppe Sep 29, 2015

Owner

Perhaps add a comment here:

// Series is serialised for backward compatibility
// as "supported-series" because a previous
// charm version had an incompatible Series field that
// was unused in practice but still serialized. This
// only applies to JSON because Meta has a custom
// YAML marshaler.

?

@wallyworld

wallyworld Sep 30, 2015

Owner

Done

meta_test.go
@@ -268,7 +268,7 @@ func (s *MetaSuite) TestSeries(c *gc.C) {
func (s *MetaSuite) TestInvalidSeries(c *gc.C) {
@rogpeppe

rogpeppe Sep 29, 2015

Owner

We should have at least one test that tests we can unmarshal a charm with multiple series.

@wallyworld

wallyworld Sep 30, 2015

Owner

We do - TestSeries()

@rogpeppe
Owner

rogpeppe commented Sep 29, 2015

LGTM with a couple of suggestions, thanks.
@wallyworld
Owner

wallyworld commented Sep 30, 2015

:shipit:
@wallyworld
Owner

wallyworld commented Sep 30, 2015

$$merge$$
@jujubot
Contributor

jujubot commented Sep 30, 2015

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju-charm

jujubot added a commit that referenced this pull request Sep 30, 2015

@jujubot
Merge pull request #158 from wallyworld/rename-supported-series-again 
Rename SupportedSeries and add json serialisation

The recent rename of Series to SupportedSeries was rejected, so we rename back.
0ba5fd4

@jujubot jujubot merged commit 0ba5fd4 into juju:v6-unstable Sep 30, 2015

@rogpeppe rogpeppe referenced this pull request Nov 6, 2015

Merged

revert to using same JSON field names as before #167

jujubot added a commit that referenced this pull request Nov 6, 2015

@jujubot
Merge pull request #167 from rogpeppe/037-correct-json-fields 
revert to using same JSON field names as before

PR #158 added explicit JSON serialisation fields but these fields were
different from before, which broke some existing code (the charm store,
for example). This leaves the explicit fields, but renames them back to what
they were previously.
fcea74e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment