-
Notifications
You must be signed in to change notification settings - Fork 80
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(td-tools): enable eslint/strict-boolean-expressions and strictNullChecks #1077
chore(td-tools): enable eslint/strict-boolean-expressions and strictNullChecks #1077
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1077 +/- ##
==========================================
- Coverage 75.34% 75.31% -0.03%
==========================================
Files 80 80
Lines 16079 16052 -27
Branches 1502 1501 -1
==========================================
- Hits 12114 12089 -25
+ Misses 3926 3924 -2
Partials 39 39
☔ View full report in Codecov by Sentry. |
Note: leave out asset-interface-description.ts because of eclipse-thingweb#1052
I started working on this, but I gave up. Thank you for bringing this up! I think we should pay double attention to the changes... because sometimes we used |
Co-authored-by: Jan Romann <[email protected]>
Co-authored-by: Jan Romann <[email protected]>
Co-authored-by: Jan Romann <[email protected]>
Co-authored-by: Jan Romann <[email protected]>
Co-authored-by: Jan Romann <[email protected]>
Co-authored-by: Jan Romann <[email protected]>
Co-authored-by: Jan Romann <[email protected]>
One more thing: Since there are lots of changes I am now more thinking about proposing PRs for each package step by step... otherwise it gets huge and hardly manageable.. |
I think after doing some more work on it I noticed this issue as well. In a sense
is not enough and we would almost need
which I really don't like... Ideas/Proposals? |
I think in this case, you can (somewhat unintuitively) cover both with |
OR "=== undefined" to "== null"
leads to different results
@JKRhb I think some of the proposed changes are not correct and lead to issues. I suggest to keep them as is... EDIT: Tests are still failing.. honestly not sure why since the only change is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some further simplifications.
Co-authored-by: Cristiano Aguzzi <[email protected]>
Co-authored-by: Cristiano Aguzzi <[email protected]>
Co-authored-by: Cristiano Aguzzi <[email protected]>
Co-authored-by: Cristiano Aguzzi <[email protected]>
Co-authored-by: Cristiano Aguzzi <[email protected]>
Co-authored-by: Cristiano Aguzzi <[email protected]>
Co-authored-by: Cristiano Aguzzi <[email protected]>
Note: I am not sure "why" but the CI gets stuck all the time in "Test with coverage report" with this PR. Local tests run fine... |
I was surprised too, let's see if #1093 get stuck too. |
Co-authored-by: Jan Romann <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greenlight from my side too!
…ullChecks (eclipse-thingweb#1077) * chore: enable eslint/strict-boolean-expressions and strictNullChecks * fix package examples * fix package td-tools Note: leave out asset-interface-description.ts because of eclipse-thingweb#1052 * Update packages/td-tools/src/td-parser.ts Co-authored-by: Jan Romann <[email protected]> * Update packages/td-tools/src/td-parser.ts Co-authored-by: Jan Romann <[email protected]> * Update packages/td-tools/src/td-parser.ts Co-authored-by: Jan Romann <[email protected]> * Update packages/td-tools/src/thing-model-helpers.ts Co-authored-by: Jan Romann <[email protected]> * Update packages/td-tools/src/thing-model-helpers.ts Co-authored-by: Jan Romann <[email protected]> * Update packages/td-tools/src/thing-model-helpers.ts Co-authored-by: Jan Romann <[email protected]> * Update packages/td-tools/src/thing-model-helpers.ts Co-authored-by: Jan Romann <[email protected]> * fix: issue issue introduced by commenting * move eslint settings to td-tools package only * fix: lint errors for AID * refactor: revert changes in package "examples" * refactor: revert "!== undefined" to "!= null" OR "=== undefined" to "== null" * refactor: revert some changes proposed by @JKRhb leads to different results * fix: wrong conversion * refactor: missing one undefined change * fix: add proper boolean check * Update packages/td-tools/src/thing-model-helpers.ts Co-authored-by: Cristiano Aguzzi <[email protected]> * Update packages/td-tools/src/util/asset-interface-description.ts Co-authored-by: Cristiano Aguzzi <[email protected]> * Update packages/td-tools/src/util/asset-interface-description.ts Co-authored-by: Cristiano Aguzzi <[email protected]> * Update packages/td-tools/src/util/asset-interface-description.ts Co-authored-by: Cristiano Aguzzi <[email protected]> * Update packages/td-tools/src/util/asset-interface-description.ts Co-authored-by: Cristiano Aguzzi <[email protected]> * Update packages/td-tools/src/util/asset-interface-description.ts Co-authored-by: Cristiano Aguzzi <[email protected]> * Update packages/td-tools/src/util/asset-interface-description.ts Co-authored-by: Cristiano Aguzzi <[email protected]> * refactor: further simplifications * Update packages/td-tools/src/td-parser.ts Co-authored-by: Jan Romann <[email protected]> * refactor: simplify data.version.model check --------- Co-authored-by: Jan Romann <[email protected]> Co-authored-by: Cristiano Aguzzi <[email protected]>
This PR addresses one checkmark in #1046.