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

chore(binding-coap): enable strict-boolean-expressions and null checks #1088

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

JKRhb
Copy link
Member

@JKRhb JKRhb commented Sep 18, 2023

This PR addresses one of the checkmarks in #1046.

@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Patch coverage: 72.72% and no project coverage change.

Comparison is base (227be53) 75.31% compared to head (fd72a5d) 75.32%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1088   +/-   ##
=======================================
  Coverage   75.31%   75.32%           
=======================================
  Files          80       80           
  Lines       16060    16057    -3     
  Branches     1503     1503           
=======================================
- Hits        12096    12095    -1     
+ Misses       3925     3923    -2     
  Partials       39       39           
Files Changed Coverage Δ
packages/binding-coap/src/coap-client.ts 82.28% <66.66%> (-0.07%) ⬇️
packages/binding-coap/src/coap-server.ts 78.55% <80.00%> (+0.18%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JKRhb JKRhb force-pushed the strict-boolean-coap branch from 091a4fa to ea9a4c8 Compare September 18, 2023 06:32
@JKRhb JKRhb added the binding-coap Issues related to coap protocol binding label Sep 18, 2023
@JKRhb JKRhb force-pushed the strict-boolean-coap branch from 8209c00 to fd72a5d Compare September 18, 2023 06:56
@JKRhb JKRhb marked this pull request as ready for review September 18, 2023 06:57
@relu91
Copy link
Member

relu91 commented Sep 20, 2023

I still didn't have proper time to review this, I want to be sure that is sufficient for check only for null and not for undefined. Given that in TS/JS world is way more common to leave values into the undefined state, I would like to be careful to only check for null

@JKRhb
Copy link
Member Author

JKRhb commented Sep 20, 2023

I still didn't have proper time to review this, I want to be sure that is sufficient for check only for null and not for undefined. Given that in TS/JS world is way more common to leave values into the undefined state, I would like to be careful to only check for null

foo != null is basically a shorthand for foo !== null && foo !== undefined – as mentioned elsewhere, this is apparently the only case where using the == operator is valid. So when checking for null this way, you get the check for undefined for free and vice versa. (Having both null and undefined has obviously not been a good language design decision 😅).

I haven't found a really good documentation link to point here, but some further explanations can be found here: https://basarat.gitbook.io/typescript/recap/null-undefined#checking-for-either

Copy link
Member

@relu91 relu91 left a comment

Choose a reason for hiding this comment

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

Silly me, I'm so used to the !== operator that I didn't even notice that you used the less strict version. Good to go.

@relu91 relu91 merged commit 95ea950 into eclipse-thingweb:master Sep 21, 2023
7 checks passed
@JKRhb JKRhb deleted the strict-boolean-coap branch September 21, 2023 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding-coap Issues related to coap protocol binding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants