Skip to content

Add default defineEndpoint in base SaloonRequest#21

Merged
Sammyjo20 merged 1 commit intosaloonphp:mainfrom
ajthinking:main
Jan 29, 2022
Merged

Add default defineEndpoint in base SaloonRequest#21
Sammyjo20 merged 1 commit intosaloonphp:mainfrom
ajthinking:main

Conversation

@ajthinking
Copy link
Copy Markdown
Contributor

Just got started with this for a graphql integration, works really good 💫
Not 100% sure this is an improvement you would like so feel free to decline it 👍

Since graphql request always targets a root like "www.acme.com/api/graphql" - no appended endpoint - would it make sense to allow to not explicitly repeating defineEndpoint method returning '' every time?

    /**
     * Define the endpoint for the request.
     *
     * @return string
     */
    public function defineEndpoint(): string
    {
        return '';
    }

This PR adds the above to the base class as a default.

Also let me know if you prefer keeping all tests in function block style. I just discovered that higher order thing 😄

Copy link
Copy Markdown
Member

@Sammyjo20 Sammyjo20 left a comment

Choose a reason for hiding this comment

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

This is a great little improvement! 🙌 Glad it is working well for GraphQL

Thank you for the PR ❤️

@Sammyjo20 Sammyjo20 merged commit f293c59 into saloonphp:main Jan 29, 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.

2 participants