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

[gql_http_link] Add response headers to context #121

Merged
merged 3 commits into from
Jul 9, 2020

Conversation

agent3bood
Copy link
Collaborator

Related issue #119

links/gql_http_link/lib/src/link.dart Outdated Show resolved Hide resolved
@@ -1,3 +1,7 @@
## 0.2.10

- add `headers` to `HttpLinkResponseContext`

Choose a reason for hiding this comment

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

I still think that this should be included on a release commit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What do you mean, should I remove this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is we update the changelog in the change PR, so this is right. @klavs seems we should probably make a more detailed contributor doc.

Copy link
Contributor

@micimize micimize left a comment

Choose a reason for hiding this comment

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

LGTM except the assert

@@ -1,3 +1,7 @@
## 0.2.10

- add `headers` to `HttpLinkResponseContext`
Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is we update the changelog in the change PR, so this is right. @klavs seems we should probably make a more detailed contributor doc.

links/gql_http_link/lib/src/link.dart Outdated Show resolved Hide resolved
@agent3bood
Copy link
Collaborator Author

@micimize What should I do now?
I am not sure if I am required to do something or if we are waiting for @klavs

@micimize
Copy link
Contributor

@agent3bood it LGTM so yeah it's just waiting on @klavs

@klavs klavs merged commit eb6f246 into gql-dart:master Jul 9, 2020
@klavs
Copy link
Contributor

klavs commented Jul 9, 2020

My understanding is we update the changelog in the change PR, so this is right. @klavs seems we should probably make a more detailed contributor doc.

@micimize, agreed. I'm not in favor of the Angular-style commit messages. That's been a major blocker for me offering small fixes to projects like graphql_flutter. I never liked spending more time reading the policies than coding the fix.

I prefer being open to "free-style" contributions with automatic checks (as much as that is possible)

@agent3bood agent3bood deleted the issues/119 branch July 9, 2020 20:02
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.

None yet

4 participants