-
Notifications
You must be signed in to change notification settings - Fork 11
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
FUSETOOLS2-2065: Provide tests for completion inside tasks.json #529
Conversation
5a9eeae
to
32512ce
Compare
3c17c2c
to
cc85e9f
Compare
cc85e9f
to
34dee98
Compare
tsconfig.json
Outdated
], | ||
"sourceMap": true, | ||
"rootDir": "src", | ||
"skipLibCheck": true, | ||
"strict": true /* enable all strict type-checking options */ |
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.
what is the reason to remove the turn strict
to false
?
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.
good point, I have forgot to mention that and discuss further the best approach..
it is mainly because we wanted to reuse same functions as in LSP without changing them, problem is that LSP project is not using strict and same tsconfig setup as DAP extension which causes problems.. so we were considering in the first iteration to reuse same functions to stay consistent for utils we are using across projects and in the next iteration we wanted to unify tsconfigs for both projects and fix utils functions which could be done in another issue/PR (already opened and selected for dev https://issues.redhat.com/browse/FUSETOOLS2-2256)
so we wrere considering options like what is better..
- for this PR revert strict change, copy functions and modify them to be compliant but in that case different from LSP ones
- remove strict for some time then fusetools2-2256 will be resolved and reuse same functions for now
in the end probably better to let strict there as is and revise everything for both projects in another mentioned issue 🤷♂️.. I can see Filip already reverted tsconfig change of strict option
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.
basically problem is that we need to
- unify tsconfigs across our extensions projects
- finally provide just one shared test utils we can share across projects to prevent these copy&paste and hard maintain issues..
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.
both solution seems reasonable
9139d1e
to
f3b536d
Compare
f3b536d
to
6747731
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Compiler changed to ES2022 to add support for .at() function on String[] to allow test parameterization.