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

Added more customisation for Fastify skipList #153

Closed
wants to merge 12 commits into from

Conversation

ozonep
Copy link

@ozonep ozonep commented Feb 17, 2021

So we can only skip specific requests (GET/POST/etc.) for specific routes.

@kibertoad
Copy link
Collaborator

@ozonep Could you please add tests for this?

@kibertoad
Copy link
Collaborator

@kobik Something appears to be wrong, CircleCI is not returning status, wonder if we need to tweak something on CircleCI panel side. Could you please either take a look or give me admin permissions for the repo so that I could take a look? Alternatively I could add GitHub actions CI instead, that one works with zero additional setup, as opposed to Travis and CircleCI.

@ozonep
Copy link
Author

ozonep commented Feb 17, 2021

Will add tests now.

@kobik
Copy link
Collaborator

kobik commented Feb 22, 2021

@kobik Something appears to be wrong, CircleCI is not returning status, wonder if we need to tweak something on CircleCI panel side. Could you please either take a look or give me admin permissions for the repo so that I could take a look? Alternatively I could add GitHub actions CI instead, that one works with zero additional setup, as opposed to Travis and CircleCI.

will check that

@kobik
Copy link
Collaborator

kobik commented Feb 22, 2021

@ozonep seems to be ok now

@kibertoad
Copy link
Collaborator

@ozonep Linting is failing, could you please fix the error?

@kibertoad kibertoad closed this Feb 22, 2021
@kibertoad kibertoad reopened this Feb 22, 2021
Copy link
Collaborator

@kibertoad kibertoad left a comment

Choose a reason for hiding this comment

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

Readme needs to be updated to describe the new structure

@ozonep
Copy link
Author

ozonep commented Feb 23, 2021

Docs updated!

@kibertoad kibertoad closed this Feb 23, 2021
@kibertoad kibertoad reopened this Feb 23, 2021
@kibertoad
Copy link
Collaborator

kibertoad commented Feb 23, 2021

@kobik This will be a semver major. Are we ready to release that already, or you'd prefer to keep current major going for a while?

@kobik
Copy link
Collaborator

kobik commented Feb 23, 2021

@kibertoad I haven't reviewed the changes, so I can't tell by myself. can't we make it somehow backwards compatible?

@kibertoad
Copy link
Collaborator

@kobik We could if you strongly prefer keeping it backwards-compatible, but that would add more conditional branching, make code more heavyweight and less performant, and would expose worse API to the end user, so I would vote against it.
It only affects fastify users and only those who use skiplist, though.

@kobik
Copy link
Collaborator

kobik commented Feb 26, 2021

@kibertoad , so a major it is.

we need to add the breaking changes note in the changelog file and upgrade procedure from the previous version

@kobik
Copy link
Collaborator

kobik commented May 12, 2021

@kibertoad i'd really like to this breaking change to be shipped with other breaking changes such as upgrading to ajv@8 in api-schema-build.

@ozonep ozonep closed this Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants