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

Update find-my-way to the latest version #1900

Closed
3 tasks done
gmahomarf opened this issue Apr 5, 2022 · 1 comment · Fixed by #1924
Closed
3 tasks done

Update find-my-way to the latest version #1900

gmahomarf opened this issue Apr 5, 2022 · 1 comment · Fixed by #1924

Comments

@gmahomarf
Copy link

  • Used appropriate template for the issue type
  • Searched both open and closed issues for duplicates of this issue
  • Title adequately and concisely reflects the feature or the bug

Feature Request

Use Case

I ran into an issue with find-my-way's routing that has since been fixed in version 5.2.0 of find-my-way. This might also fix #1811, #1873 and #1874

Example API

N/A

Are you willing and able to implement this?

I can open a PR updating the version, but there are probably tests that need to be run and checks that need to happen. Based on find-my-way's changelog, breaking changes include dropped support for Node.js < 12, and I don't know what the officially supported version for Restify is

@rektide
Copy link
Contributor

rektide commented Sep 23, 2022

I ran into some assertion errors when running tests with newer versions of find-my-way. 3.0.5 passed tests, but 4.0.0 failed with four assertions, each where we are comparing printed-out route maps from find-my-way. There's new syntax we don't seem to have: └── / (GET|POST)\n, where one route is both GET and POST. I'm not 100% the route maps really are 100% the same quite yet, but am analyzing further, and cautiously optimistic: I tentatively think this is just that find-my-way has changed how it outputs/prints it's route map.

I do hope to have a PR up at some point. I've dug through the changelog & there don't seem to be any obvious significant changes. I think it may just be this route printing. Hopefully I'll have a PR up soon.

As you say, we will need to upgrade node. There are issues #1921 & pr #1923 for that.

An upgrade would also resolve #1868. Thanks for the list of other issues this would resolve @gmahomarf.

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 a pull request may close this issue.

2 participants