Skip to content

fix: remove http version check#4962

Merged
Uzlopak merged 17 commits intofastify:nextfrom
beyazit:http-version-check
Aug 29, 2023
Merged

fix: remove http version check#4962
Uzlopak merged 17 commits intofastify:nextfrom
beyazit:http-version-check

Conversation

@beyazit
Copy link
Contributor

@beyazit beyazit commented Aug 8, 2023

Checklist

Node.js issue #43115 has been resolved so I have removed this check.

@jsumners
Copy link
Member

jsumners commented Aug 8, 2023

I suspect this is a major version change and should thus target the next branch.

@beyazit beyazit changed the base branch from main to next August 8, 2023 23:31
@beyazit
Copy link
Contributor Author

beyazit commented Aug 8, 2023

I suspect this is a major version change and should thus target the next branch.

I changed pull request base to next branch.

Copy link
Member

@climba03003 climba03003 left a comment

Choose a reason for hiding this comment

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

I changed pull request base to next branch.

You need to rebase your changes on top of the next branch, so it would not include all the unnecessary commits.

@climba03003 climba03003 added semver-major Issue or PR that should land as semver major v5.x Issue or pr related to Fastify v5 labels Aug 9, 2023
cm-ayf and others added 13 commits August 9, 2023 15:25
* fix: FastifySchemaValidationError.params could be unknown

* refactor: use FastifySchemaValidationError instead of ValidationResult

* fix: export ValidationResult for backword compatibility

* test: add non-assignable test for AjvErrorObject
Co-authored-by: Carlos Fuentes <me@metcoder.dev>
* docs: early hints plugin is fastify plugin

* fix

* Update docs/Guides/Ecosystem.md

Co-authored-by: Frazer Smith <frazer.dev@outlook.com>

---------

Co-authored-by: Frazer Smith <frazer.dev@outlook.com>
* chore: add pull request title check

* Update .github/workflows/pull-request-title.yml

Co-authored-by: Frazer Smith <frazer.dev@outlook.com>

---------

Co-authored-by: Frazer Smith <frazer.dev@outlook.com>
Bumps [@sinclair/typebox](https://github.com/sinclairzx81/typebox) from 0.29.6 to 0.30.2.
- [Commits](sinclairzx81/typebox@0.29.6...0.30.2)

---
updated-dependencies:
- dependency-name: "@sinclair/typebox"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* improve pr title check

* Apply suggestions from code review

* fix

* fix

* Update .github/workflows/pull-request-title.yml

Co-authored-by: Luis Orbaiceta <luisorbaiceta@gmail.com>

* Apply suggestions from code review

---------

Co-authored-by: Luis Orbaiceta <luisorbaiceta@gmail.com>
* improve pr title check

* Apply suggestions from code review

* fix

* fix

* ci: fix warnings in benchmark workflow

* Update .github/workflows/benchmark.yml

* Update .github/workflows/benchmark.yml
* docs(Validator Compiler): fix removeAdditional comment

* Update docs/Reference/Validation-and-Serialization.md

Co-authored-by: Uzlopak <aras.abbasi@googlemail.com>

* fix: typo in comment

* Update docs/Reference/Validation-and-Serialization.md

Co-authored-by: Frazer Smith <frazer.dev@outlook.com>

---------

Co-authored-by: Uzlopak <aras.abbasi@googlemail.com>
Co-authored-by: Frazer Smith <frazer.dev@outlook.com>
* fix: infer correct hook handler

* Update test/types/hooks.test-d.ts

* simplify

* remove duplicate typing tests

* remove duplicate test

* remove unused import

* fix

* fix linting
@beyazit
Copy link
Contributor Author

beyazit commented Aug 9, 2023

damn, is this correct? i rebased my commits to fastify/next and made a force push

git checkout http-version-check
git remote add upstream https://github.com/fastify/fastify.git
git fetch upstream
git rebase upstream/next
git push origin next --force

@beyazit beyazit requested a review from climba03003 August 9, 2023 12:37
@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 10, 2023

I merged main into next and updated this branch. The commits are still the same ,but a squash merge should fix all the issues.

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 21, 2023

@climba03003
PTAL

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 23, 2023

Maybe should also remove

https://github.com/fastify/fastify/blob/main/docs/Reference/Routes.md#--http-version-check

@climba03003

Well, lets say the underlying PR got merged into node code. Is it safe to remove this check already?

@climba03003
Copy link
Member

I still prefer a clean history even on PR.

Maybe should also remove

Yes

Is it safe to remove this check already?

Yes, I have double checked the changes is landed on 16, 18 and 20.

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 23, 2023

@beyazit

Are you still actively working on this PR?

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 29, 2023

@climba03003

I guess, @beyazit abandoned this PR.

Copy link
Contributor

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

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

LGTM

@Uzlopak Uzlopak merged commit dbac4e0 into fastify:next Aug 29, 2023
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

semver-major Issue or PR that should land as semver major v5.x Issue or pr related to Fastify v5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants