Skip to content

Serialize Map types in GraphQLQuery#340

Merged
berngp merged 2 commits intoNetflix:masterfrom
richardcresswell:serialize-input-maps
May 14, 2021
Merged

Serialize Map types in GraphQLQuery#340
berngp merged 2 commits intoNetflix:masterfrom
richardcresswell:serialize-input-maps

Conversation

@richardcresswell
Copy link
Copy Markdown
Contributor

Pull request checklist

  • Please read our contributor guide
  • Consider creating a discussion on the discussion forum
    first
  • Make sure the PR doesn't introduce backward compatibility issues
  • Make sure to have sufficient test cases

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Other (please describe):

Changes in this PR

Issue #290

Support Maps in GraphQLQuery serialization. This is useful for JSON and Object scalars.

Alternatives considered

n/a

@richardcresswell
Copy link
Copy Markdown
Contributor Author

Did the CI checks have some kind of environmental issue not related to this pull request?

@paulbakker
Copy link
Copy Markdown
Collaborator

Did the CI checks have some kind of environmental issue not related to this pull request?

Yes, I think so. Several other builds also failed.

@paulbakker
Copy link
Copy Markdown
Collaborator

@richardcresswell I've restarted the builds, fingers crossed that just fixes it

@berngp
Copy link
Copy Markdown
Contributor

berngp commented May 14, 2021

I'm looking at the build failures. Will disable test-reporting for now to get this going.

@berngp
Copy link
Copy Markdown
Contributor

berngp commented May 14, 2021

@richardcresswell could you please rebase against master?

@richardcresswell
Copy link
Copy Markdown
Contributor Author

Should be all set.

@berngp
Copy link
Copy Markdown
Contributor

berngp commented May 14, 2021

@paulbakker @richardcresswell boggles the mind that the CI Workflows are still trying to Publish the Tests. This was disabled and both master as well as @richardcresswell 's branch reflect it.

@berngp
Copy link
Copy Markdown
Contributor

berngp commented May 14, 2021

The code changes look fine and and the project build as well. The failures are based on the failure to publish the JUnit Reports.
There is a caveat on using the pull_request action that I was not aware and affects the publishing of such reports, if the PR comes from an external fork.

Ref.

  1. https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/
  2. https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

@berngp
Copy link
Copy Markdown
Contributor

berngp commented May 14, 2021

@paulbakker @richardcresswell , the build is green.

@berngp berngp merged commit 0dd2a8b into Netflix:master May 14, 2021
@berngp
Copy link
Copy Markdown
Contributor

berngp commented May 14, 2021

Thanks @richardcresswell for the PR and patience with the build.

@richardcresswell
Copy link
Copy Markdown
Contributor Author

Thanks for merging! So we can plan on our side, do you know when you will be publishing the next RC or release?

@richardcresswell richardcresswell deleted the serialize-input-maps branch May 18, 2021 15:59
@srinivasankavitha
Copy link
Copy Markdown
Contributor

@richardcresswell - we will release this week, likely in the next day or two.

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.

4 participants