feat: add WithHeader & WithCookie

[Laisky]

can set http client's headers and cookies

cli := NewClient(
    "url",
    httpClient,
    graphql.WithCookie("cookieName", "cookieVal"),
    graphql.WithHeader("headerName", "headerVal"),
)

fully backwards-compatible.

[IamFlowZ]

Will this be merged in anytime soon? Would be specifically useful for authorization.

[Laisky]

@IamFlowZ you can try my fork github.com/Laisky/graphql, fully compatible with shurcool/graphql.

https://github.com/Laisky/graphql

[t829702]

Could you @Laisky not include unnecessary changes like this?

[Laisky]

done

[dhawton]

Changing a function's signature is a very bad idea for backward compatibility. It's better to create a new function and have the old call the new. A blog entry from the Go team discusses this here: https://blog.golang.org/module-compatibility

[Laisky]

just add some option args, not change the function's behavior.

[dhawton]

Correct, you changed the function's signature. By adding args to the function, you broke backward compatibility. See the blog post from the Go team that I linked in the original message. You should never change a function's signature in a module unless you're doing a major version release with breaking changes, and even then, try to avoid it.

[Laisky]

you broke backward compatibility

it not broke the backward compatibility. I add option args, you can just ignore these args.

[Seklfreak]

The change here is to add a variadic, old implementations will not break. How is this a breaking change?

[dhawton]

Laisky, again, you changed the function's signature. Please see the provided reference on how it is a breaking change. The blog post from the Go team explains how it's a breaking change when you add args to an exported function. From the provided link "Often, breaking changes come in the form of new arguments to a function." https://blog.golang.org/module-compatibility they also cover how to add arguments to an exported function without making it a breaking change. Don't change a function's signature if you don't need to..

[Laisky]

I add an new method called NewClientWithOptions

[StevenACoffman]

This is a great feature, but I would also prefer it if these were on a new method like NewClientWithOptions or something.

[Laisky]

This is a great feature, but I would also prefer it if these were on a new method like NewClientWithOptions or something.

good idea

[Laisky]

I add an new method called NewClientWithOptions

[StevenACoffman]

Thanks! I had a minor suggestion. (also, I'm not a maintainer, but would love to see this merged)

[andig]

Not a 1:1 replacement, but #73 might also enable this use case

[khwerhahn]

When is this going to be merged?

[jaskirat8]

will this be merged ?