-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
add '_scope' to task configuration #4452
Conversation
c84d544
to
eec0a97
Compare
eec0a97
to
f53de3a
Compare
@@ -33,15 +32,12 @@ export class QuickOpenTask implements QuickOpenModel, QuickOpenHandler { | |||
@inject(TaskService) | |||
protected readonly taskService: TaskService; | |||
|
|||
@inject(TaskConfigurations) | |||
protected readonly taskConfigurations: TaskConfigurations; |
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.
that's 0.5.0
i'm fine with that, please be conscious when you break
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.
you are right. i will put it back. Thank you !
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.
There are 2 more breaking changes in this PR.
I'm fine with 0.5.0 for next. There are PRs, like refactoring git to scm extension, for which it is not avoidable.
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.
@akosyakov just thinking about API breaking going under the radar, would there be a strategy to adopt in order to catch any API breakage?
I guess this could be done with our test suite, but despite all the tests we have, it seems to not be enough. Is there something that should be done?
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.
I'm not aware of any tools for TS allowing to check it. Even if there are I am not sure that we want to use them, since we are fine with breaking and updating the major version. It would cause a lot of noise.
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.
I put taskConfigurations
back and commented To be removed in 0.5.0
. We could leave it here for the release where other breaking changes are made. What do you think ?
@elaihau Could you describe how one can test it? Do you have a repo? |
To align the task list display in theia with that of vscode, we need to differentiate 3 things: - the task type - where the task comes from, and - where the task is supposed to be run from This change adds an optional '_scope' to the TaskConfiguration interface. Together with 'type' and '_source', we have enough number of properties to hold the information from vscode extensions. Signed-off-by: elaihau <liang.huang@ericsson.com>
@akosyakov you are right Anton, I should have asked more people to test it before merge. |
To align the task list display in theia with that of vscode, we need to differentiate 3 things:
This change adds an optional '_scope' to the TaskConfiguration interface. Together with 'type' and '_source', we have enough number of properties to hold the information from vscode extensions.
Signed-off-by: elaihau liang.huang@ericsson.com