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

docs: clarify apollo codegen usage #1428

Closed
wants to merge 1 commit into from

Conversation

jorroll
Copy link

@jorroll jorroll commented Jul 21, 2019

This PR updates the packages/apollo/README.md file to clarify the usage/limitations of apollo codegen. It's intended as a partial fix / quick improvement for #1427.

cc @JakeDawkins

@JakeDawkins
Copy link
Contributor

@thefliik Thanks for this, and sorry for this, but I don't think this is the best approach for handling this for a couple reasons.

First, the section of the docs where you added your comments is autogenerated, so it would get overwritten. We don't have a way to add more than a command's description inline with the command's usage docs right now.

Secondly, the fact that codegen can be only run on a single project, not at a monorepo's root isn't unique to codegen. The whole CLI and all commands are designed currently to only operate on a single project. We do hope to change this with the next major version of the CLI though.

I think it could be useful to add an explicit note on the configuration docs, but I'm unsure.

As much as it's imperfect right now, I think the bigger win would be to adjust the warning that's produced with duplicate operation names to mention that types were not generated, rather than add these docs in here.

@jorroll
Copy link
Author

jorroll commented Jul 25, 2019

the section of the docs where you added your comments is autogenerated

Ah. That does explain the strange formatting as well.

As much as it's imperfect right now, I think the bigger win would be to adjust the warning that's produced with duplicate operation names to mention that types were not generated, rather than add these docs in here.

That's definitely the bigger win, but as you said over in #670.

We're also pretty heavily backlogged on codegen issues at the moment, so I'm not sure how quickly something like this could be added/reviewed.

An imperfect docs update is better than the current situation (i.e. nothing). This being said, I'm not certain I've ever seen those configuration docs you linked to (though it seems likely I looked at them when I configured the Apollo VS Code extension). In general, I imagine someone looking for information on proper Apollo CLI usage will come here.

but I don't think this is the best approach for handling this for a couple reasons

Correct me if I'm wrong, but I'm getting the impression that your preferred approach is "do nothing and wait for a PR addressing #1427 to be merged in" ? That's not a good solution as I imagine addressing #1427 could take quite a while.

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

2 participants