Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

fix(status): fix Constraint & Version json output #976

Merged
darkowlzz merged 4 commits into from
Aug 13, 2017
Merged

fix(status): fix Constraint & Version json output #976

darkowlzz merged 4 commits into from
Aug 13, 2017

Conversation

@darkowlzz
Copy link
Collaborator

@darkowlzz darkowlzz commented Aug 8, 2017 •
edited
Loading

What does this do / why do we need it?

Replace Constraint & Version with their json alternative forms JSONConstraint and JSONVersion.
Constraint and Version are always empty now.

dep status on golang/dep,
before:

$ dep status -json | jq
[
  {
    "ProjectRoot": "github.com/Masterminds/semver",
    "Children": null,
    "Constraint": {},
    "Version": {},
    "Revision": "139cc0982c53f1367af5636b12b7643cd03757fc",
    "Latest": "c2e7f6c2f49a1613362aa774884c17e395dd85b7",
    "PackageCount": 1
  },
  {
    "ProjectRoot": "github.com/Masterminds/vcs",
    "Children": null,
    "Constraint": {},
    "Version": {},
    "Revision": "3084677c2c188840777bff30054f2b553729d329",
    "Latest": "3084677c2c188840777bff30054f2b553729d329",
    "PackageCount": 1
  },
  {
    "ProjectRoot": "github.com/armon/go-radix",
    "Children": null,
    "Constraint": {},
    "Version": {},
    "Revision": "4239b77079c7b5d1243b7b4736304ce8ddb6f0f2",
    "Latest": "1fca145dffbcaa8fe914309b1ec0cfc67500fe61",
    "PackageCount": 1
  },
  {
    "ProjectRoot": "github.com/go-yaml/yaml",
    "Children": null,
    "Constraint": {},
    "Version": {},
    "Revision": "cd8b52f8269e0feb286dfeef29f8fe4d5b397e0b",
    "Latest": "25c4ec802a7d637f88d584ab26798e94ad14c13b",
    "PackageCount": 1
  },
  {
    "ProjectRoot": "github.com/nightlyone/lockfile",
    "Children": null,
    "Constraint": {},
    "Version": {},
    "Revision": "e83dc5e7bba095e8d32fb2124714bf41f2a30cb5",
    "Latest": "6a197d5ea61168f2ac821de2b7f011b250904900",
    "PackageCount": 1
  },
  {
    "ProjectRoot": "github.com/pelletier/go-buffruneio",
    "Children": null,
    "Constraint": {},
    "Version": {},
    "Revision": "c37440a7cf42ac63b919c752ca73a85067e05992",
    "Latest": "c37440a7cf42ac63b919c752ca73a85067e05992",
    "PackageCount": 1
  },
  {
    "ProjectRoot": "github.com/pelletier/go-toml",
    "Children": null,
    "Constraint": {},
    "Version": {},
    "Revision": "fe206efb84b2bc8e8cfafe6b4c1826622be969e3",
    "Latest": "69d355db5304c0f7f809a2edc054553e7142f016",
    "PackageCount": 1
  },
  {
    "ProjectRoot": "github.com/pkg/errors",
    "Children": null,
    "Constraint": {},
    "Version": {},
    "Revision": "645ef00459ed84a119197bfb8d8205042c6df63d",
    "Latest": "645ef00459ed84a119197bfb8d8205042c6df63d",
    "PackageCount": 1
  },
  {
    "ProjectRoot": "github.com/sdboyer/constext",
    "Children": null,
    "Constraint": {},
    "Version": {},
    "Revision": "836a144573533ea4da4e6929c235fd348aed1c80",
    "Latest": "836a144573533ea4da4e6929c235fd348aed1c80",
    "PackageCount": 1
  }
]

with this change:

$ dep status -json | jq
[
  {
    "ProjectRoot": "github.com/Masterminds/semver",
    "Children": null,
    "Revision": "139cc0982c53f1367af5636b12b7643cd03757fc",
    "Latest": "c2e7f6c2f49a1613362aa774884c17e395dd85b7",
    "PackageCount": 1,
    "Constraint": "branch 2.x",
    "Version": "branch 2.x"
  },
  {
    "ProjectRoot": "github.com/Masterminds/vcs",
    "Children": null,
    "Revision": "3084677c2c188840777bff30054f2b553729d329",
    "Latest": "3084677c2c188840777bff30054f2b553729d329",
    "PackageCount": 1,
    "Constraint": "^1.11.0",
    "Version": "v1.11.1"
  },
  {
    "ProjectRoot": "github.com/armon/go-radix",
    "Children": null,
    "Revision": "4239b77079c7b5d1243b7b4736304ce8ddb6f0f2",
    "Latest": "1fca145dffbcaa8fe914309b1ec0cfc67500fe61",
    "PackageCount": 1,
    "Constraint": "*",
    "Version": "branch master"
  },
  {
    "ProjectRoot": "github.com/go-yaml/yaml",
    "Children": null,
    "Revision": "cd8b52f8269e0feb286dfeef29f8fe4d5b397e0b",
    "Latest": "25c4ec802a7d637f88d584ab26798e94ad14c13b",
    "PackageCount": 1,
    "Constraint": "branch v2",
    "Version": "branch v2"
  },
  {
    "ProjectRoot": "github.com/nightlyone/lockfile",
    "Children": null,
    "Revision": "e83dc5e7bba095e8d32fb2124714bf41f2a30cb5",
    "Latest": "6a197d5ea61168f2ac821de2b7f011b250904900",
    "PackageCount": 1,
    "Constraint": "*",
    "Version": "branch master"
  },
  {
    "ProjectRoot": "github.com/pelletier/go-buffruneio",
    "Children": null,
    "Revision": "c37440a7cf42ac63b919c752ca73a85067e05992",
    "Latest": "c37440a7cf42ac63b919c752ca73a85067e05992",
    "PackageCount": 1,
    "Constraint": "*",
    "Version": "v0.2.0"
  },
  {
    "ProjectRoot": "github.com/pelletier/go-toml",
    "Children": null,
    "Revision": "fe206efb84b2bc8e8cfafe6b4c1826622be969e3",
    "Latest": "69d355db5304c0f7f809a2edc054553e7142f016",
    "PackageCount": 1,
    "Constraint": "branch master",
    "Version": "branch master"
  },
  {
    "ProjectRoot": "github.com/pkg/errors",
    "Children": null,
    "Revision": "645ef00459ed84a119197bfb8d8205042c6df63d",
    "Latest": "645ef00459ed84a119197bfb8d8205042c6df63d",
    "PackageCount": 1,
    "Constraint": "^0.8.0",
    "Version": "v0.8.0"
  },
  {
    "ProjectRoot": "github.com/sdboyer/constext",
    "Children": null,
    "Revision": "836a144573533ea4da4e6929c235fd348aed1c80",
    "Latest": "836a144573533ea4da4e6929c235fd348aed1c80",
    "PackageCount": 1,
    "Constraint": "*",
    "Version": "branch master"
  }
]

What should your reviewer look out for in this PR?

Implementation correctness.

Do you need help or clarification on anything?

No.

Which issue(s) does this PR fix?

No open issue.

@sdboyer
Copy link
Member

sdboyer commented Aug 8, 2017

could you say a little more about what larger problem this is fixing? just so i can keep up with what all is happening

@darkowlzz
Copy link
Collaborator Author

darkowlzz commented Aug 8, 2017 •
edited
Loading

oops! sorry 😅 yes, so in json output, Constraint and Version would always be empty. Even though other formats would have values for them, json would return empty {} for constraint and version.

Updated the first comment with output examples to make it easy to understand 😊

@darkowlzz darkowlzz changed the title fix(status): fix Constraint & Version output [WIP] fix(status): fix Constraint & Version output Aug 9, 2017
@darkowlzz darkowlzz changed the title [WIP] fix(status): fix Constraint & Version output [WIP] fix(status): fix Constraint & Version json output Aug 9, 2017
@darkowlzz darkowlzz force-pushed the status-constraint-version-fix branch from 53a432e to afb281e Compare August 10, 2017 16:07
@darkowlzz darkowlzz changed the title [WIP] fix(status): fix Constraint & Version json output fix(status): fix Constraint & Version json output Aug 10, 2017
@darkowlzz
Copy link
Collaborator Author

darkowlzz commented Aug 10, 2017 •
edited
Loading

Added table tests for BasicLine output for different output types.

Also added getConsolidatedConstraint() and getConsolidatedVersion() to have common function for constraint and version string formation, in all the different types of outputs.

sdboyer
sdboyer reviewed Aug 10, 2017
View reviewed changes
Copy link
Member

@sdboyer sdboyer left a comment

Choose a reason for hiding this comment

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

instead of adding new field types, what if we implement MarshalJSON and do the conversions there directly (perhaps using an intermediate type, like we do with Manifest.MarshalTOML?

this strikes me as preferable to having duplicate fields in the main struct itself. thoughts?

cmd/dep/status_test.go Outdated
}

func TestBasicStatusGetConsolidatedConstraint(t *testing.T) {

Copy link
Member

@sdboyer sdboyer Aug 10, 2017

Choose a reason for hiding this comment

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

nit: unnecessary newline

@darkowlzz
fix(status): fix Constraint & Version output
61e4224 
Replace `Constraint` & `Version` with their json alternative forms
`JSONConstraint` and `JSONVersion`.
@darkowlzz
test(status): BasicLine output & BasicStatus func
e00c4ff 
Add table tests for BasicLine output for each output type and tests for
getConsolidatedConstraint() & getConsolidatedVersion().
@darkowlzz darkowlzz force-pushed the status-constraint-version-fix branch from afb281e to bd3ba74 Compare August 12, 2017 14:23
@darkowlzz
Copy link
Collaborator Author

darkowlzz commented Aug 12, 2017

Added marshalJSON() with rawStatus struct.

Also, Children is not part of rawStatus. I thought that's not required. Correct me if that's required :)

$ dep status -json | jq
[
  {
    "ProjectRoot": "github.com/Masterminds/semver",
    "Constraint": "branch parse-constraints-with-dash-in-pre",
    "Version": "branch parse-constraints-with-dash-in-pre",
    "Revision": "a93e51b5a57ef416dac8bb02d11407b6f55d8929",
    "Latest": "a93e51b5a57ef416dac8bb02d11407b6f55d8929",
    "PackageCount": 1
  },
  {
    "ProjectRoot": "github.com/Masterminds/vcs",
    "Constraint": "^1.11.0",
    "Version": "v1.11.1",
    "Revision": "3084677c2c188840777bff30054f2b553729d329",
    "Latest": "3084677c2c188840777bff30054f2b553729d329",
    "PackageCount": 1
  },
  {
    "ProjectRoot": "github.com/armon/go-radix",
    "Constraint": "*",
    "Version": "branch master",
    "Revision": "4239b77079c7b5d1243b7b4736304ce8ddb6f0f2",
    "Latest": "1fca145dffbcaa8fe914309b1ec0cfc67500fe61",
    "PackageCount": 1
  },
...
]

@darkowlzz
fix(status): add marshalJSON()
9300b62 
Add `rawStatus` struct and use `marshalJSON()` to convert `BasicStatus`
to `rawStatus`.
@darkowlzz
test(status): consolidated constraint with override
c6b462b
@darkowlzz darkowlzz force-pushed the status-constraint-version-fix branch from bd3ba74 to c6b462b Compare August 12, 2017 16:09
@sdboyer
Copy link
Member

sdboyer commented Aug 13, 2017

yeah, i...don't even remember the original motivation for Children 😄

this LGTM - merge at your convenience!

@darkowlzz darkowlzz merged commit b0e3b27 into golang:master Aug 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@sdboyer sdboyer sdboyer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@darkowlzz @sdboyer @googlebot