search

mjarkk / vanmoof-web-controller

Change the speed limit of a VanMoof S3 or X3 via a website
https://mooovy.app
GNU Lesser General Public License v2.1
79 stars 14 forks source link

Lots of changes after #10 was merged #15

Closed mjarkk closed 2 years ago

mjarkk commented 2 years ago

@sanderDijkxhoorn here are some last things i wanted different in your pr but i didn't feel the need to comment on them as they where do not have high priority and would have bloated the pr's review process. Beside that i didn't want to overwhelm you with a fuckload of comments :) That said you might want to look into the changes to see what i've would do differently

vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
vanmoof-web-controller ✅ Ready (Inspect) Visit Preview Jun 23, 2022 at 9:53AM (UTC)
sanderDijkxhoorn commented 2 years ago

Why did you change the next.config.js?

sanderDijkxhoorn commented 2 years ago

Why is createBikeSharingInvitation() a Promise<void>?

Void means it doesn’t return anything right?

mjarkk commented 2 years ago

Why did you change the next.config.js?

I cannot obtain my bike shares without the change you made earlier so it seems like this change is needed

Why is createBikeSharingInvitation() a Promise?

If a function in async it always returns a Promise hence why I've added that

Void means it doesn’t return anything right?

What is returning nothing? In lower level languages like C++ you have void types for functions that latterly return nothing. In javascript a function always returns something you can call a empty function and you won't get void but undefined where undefined is valid javascript a value.

With the changes I made to the error checking we don't need the response value anymore, we only have to check if the function succeeds or if it throws thus no need anymore for the response.

As the response value is not important I have labeled the response as void hinting the user of the method "you can ignore the response value"

I think this is a fine usage of the void type, reading the typescript docs it says:

image

Tough that all said this is a bit of a grey area and if we want to be fully compliant with typescript we should use empty return statements.

sanderDijkxhoorn commented 2 years ago

I cannot obtain my bike shares without the change you made earlier so it seems like this change is needed

have you tried to restart the npm run dev when my PR got approved? The api was working fine for me.

mjarkk commented 2 years ago

Yes I've tried that

sanderDijkxhoorn commented 2 years ago

Hmm weird…