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

GraphManager/Engine Headers + minor WebSocket cleanup #858

Merged
merged 14 commits into from
Oct 29, 2019

Conversation

designatednerd
Copy link
Contributor

In this PR:

  • Added default and ability to set non-default name and version headers for Apollo GraphManager and Apollo Engine
  • Pulled some web socket components out of the giant class file into their own classes, start adding inline documentation (to the extent that I understand what's happening 🙃).
  • Added a convenience extension for getting stuff out of a bundle's info dictionary.

if version.isEmpty {
version.append(buildNumber)
} else {
version.append("-\(buildNumber)")
Copy link
Contributor

Choose a reason for hiding this comment

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

I actually decided against this on my end as I'm increasing the build number on each build (based of number of commits) and it makes the "Clients" view in Apollo Engine a bit noisy for the non-public environments/variants. YMMV of course and it's always configurable anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah in my past experience when I've had build number auto-incrementing it's actually been quite helpful in being able to identify exactly where I broke something 😄. And as you said, it's configurable, so if people don't like the default, they can always adjust.

@designatednerd designatednerd merged commit d3fc367 into master Oct 29, 2019
@designatednerd designatednerd deleted the add/headery-fun branch October 29, 2019 20:28
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.

2 participants