search

diia-open-source / be-pkg-utils

European Union Public License 1.2
14 stars 10 forks source link

My favorite terrible place #19

Open tshemsedinov opened 4 months ago

tshemsedinov commented 4 months ago
  1. Union type number | undefined is not ok here, why should we pass undefined here?
  2. Hardcoded http error codes without names, without named constants
  3. Expression code >= 100 && code < 600 in if statement returns boolean itself, so we do not need to use return true/return false
  4. Where can we use isHttpCode? Code range have gaps, so this will not validate, see RFC http for reference https://github.com/diia-open-source/be-pkg-utils/blob/e969374fb769e59ab6bbdb709caeba0199e14a8c/src/network.ts#L50-L60
tshemsedinov commented 4 months ago

Same problem here: https://github.com/diia-open-source/be-pkg-utils/blob/e969374fb769e59ab6bbdb709caeba0199e14a8c/src/network.ts#L62-L68

vird commented 4 months ago

Just as an idea, needs reworking to be more matched with code style https://github.com/diia-open-source/be-pkg-utils/pull/26

tshemsedinov commented 4 months ago

@vird Node.js have status codes collection: https://nodejs.org/api/http.html#httpstatus_codes

vird commented 4 months ago

I'm not sure we can use that here. Didn't investigate if any other packages are trying to use this one at frontend

tshemsedinov commented 4 months ago

be-* package names is backend @vird

vird commented 4 months ago

Agree But with webpack any package which was previously named "for backend" can be used on frontend We have no information about non-released parts of diia