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

Replace ambiguous code example in the documentation #178

Merged
merged 1 commit into from
May 31, 2022

Conversation

ryanzidago
Copy link

@ryanzidago ryanzidago commented Jan 13, 2022

Hi all,

First thing first, thanks for this library and the tutorial.

The current wording is confusing: it says that the lines should be "added" to the Application.module function, but the code example underneath shows you the whole function, without the configureRouting() call which is necessary to access the graphql playground.

IMHO, we should simply tell the reader to either:

  • replace the current Application.module function with whatever is displayed as a code example underneath (what I suggest in this PR)
  • reformat the code example so as to make sure that this change is to be added to the Application.module and not to replace it for example:
fun Application.module(testing: Boolean = false) {
    // ...
    install(GraphQL) {
        playground = true
        schema { 
            query("hello") {
                resolver { -> "World" }
            }
        }
    }
}

In this PR, I chose the first option, as it is the most straightforward, less error prone and easy to follow.

@ryanzidago ryanzidago changed the title Update ktor.md Replace ambiguous code example in the documentation Jan 13, 2022
@sangeetds
Copy link

Hi @ryanzidago
I think this would be a good update to the README.md. I have forked this repo and trying to maintain it. I thought it would be better if you could open the same PR at https://github.com/sangeetds/KGraphQL instead of me pushing your commit from there. Thanks!

@jeggy
Copy link
Member

jeggy commented May 31, 2022

I'm approving this for now. But we should upgrade the ktor plugin the be available for ktor version 2.x and then update the tutorial to be written for ktor 2.x and using the new ktor project wizard.

@jeggy jeggy merged commit 4187dc8 into aPureBase:main May 31, 2022
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.

3 participants