Skip to content

Conversation

@crosbymichael
Copy link
Member

Fixes #362

Signed-off-by: Michael Crosby [email protected]

@wking
Copy link
Contributor

wking commented Mar 31, 2016 via email

@JakeWarner
Copy link

LGTM

@vbatts
Copy link
Member

vbatts commented Mar 31, 2016

so many lines.
@crosbymichael is there any other way you can still benefit from the unmarshal speed improvements?

@crosbymichael
Copy link
Member Author

not that i know of at this point

@vbatts
Copy link
Member

vbatts commented Mar 31, 2016

i wonder if having a build tag + go:generate would allow a way around incompatible things like gccgo?

@wking
Copy link
Contributor

wking commented Mar 31, 2016

On Thu, Mar 31, 2016 at 11:52:21AM -0700, Michael Crosby wrote:

not that i know of at this point

The cludgy approach would be “fork this repo and keep ffjson in the
fork” (this could maybe happen in a separate long-running branch of
this repo, but I'm not clear enough on Go vendoring to know). Then
consumers could pick whichever implementation matched their preferred
performance / portability balance. With the ffjson being
autogenerated, maintaining the fork shouldn't be too much work.

@vbatts
Copy link
Member

vbatts commented Mar 31, 2016

@wking noup

@wking
Copy link
Contributor

wking commented Mar 31, 2016

On Thu, Mar 31, 2016 at 12:00:56PM -0700, Vincent Batts wrote:

i wonder if having a build tag + go:generate would allow a way
around incompatible things like gccgo?

This also sounds promising to me, and would certainly be cleaner than
using two branches/repos if it works out.

@JakeWarner
Copy link

Personally, I'd wait to potentially re-integrate ffjson until the issue behind pquerna/ffjson#172 is resolved as any golang1.6 builds aren't consistent due to the spec having anonymous structs. I see that @crosbymichael closed #361 due to this pull request.

@crosbymichael
Copy link
Member Author

ya, i think we can remove for now and work with upstream to fix some of the issues that we have

@vbatts
Copy link
Member

vbatts commented Mar 31, 2016

sounds fine.

LGTM

@mrunalp
Copy link
Contributor

mrunalp commented Mar 31, 2016

LGTM

@mrunalp mrunalp merged commit d2de3d9 into opencontainers:master Mar 31, 2016
@crosbymichael crosbymichael deleted the revert-ffjson branch March 31, 2016 20:29
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Apr 12, 2016
Through 6734c7a (Merge pull request opencontainers#370 from
vbatts/json_schema_and_examples, 2016-04-11).

The only unlisted changes to master were a brief run with ffjson
(opencontainers#343, opencontainers#351), but that was pulled out due to gccgo issues in opencontainers#363.

Signed-off-by: W. Trevor King <[email protected]>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Apr 12, 2016
Through 6734c7a (Merge pull request opencontainers#370 from
vbatts/json_schema_and_examples, 2016-04-11).

The only unlisted changes to master were a brief run with ffjson
(opencontainers#343, opencontainers#351), but that was pulled out due to gccgo issues in opencontainers#363.

Signed-off-by: W. Trevor King <[email protected]>
vbatts pushed a commit to vbatts/oci-runtime-spec that referenced this pull request Apr 12, 2016
Through 6734c7a (Merge pull request opencontainers#370 from
vbatts/json_schema_and_examples, 2016-04-11).

The only unlisted changes to master were a brief run with ffjson
(opencontainers#343, opencontainers#351), but that was pulled out due to gccgo issues in opencontainers#363.

Signed-off-by: W. Trevor King <[email protected]>
Mashimiao pushed a commit to Mashimiao/specs that referenced this pull request Aug 19, 2016
Through 6734c7a (Merge pull request opencontainers#370 from
vbatts/json_schema_and_examples, 2016-04-11).

The only unlisted changes to master were a brief run with ffjson
(opencontainers#343, opencontainers#351), but that was pulled out due to gccgo issues in opencontainers#363.

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.

5 participants