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.

Sign up for GitHub

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

Jump to bottom

Canceling a progress group doesn't cancel the monitors inside that group #664

Closed
fedejeanne opened this issue Sep 7, 2023 · 0 comments · Fixed by #666
Closed

Canceling a progress group doesn't cancel the monitors inside that group #664

fedejeanne opened this issue Sep 7, 2023 · 0 comments · Fixed by #666

Comments

@fedejeanne
Copy link
Contributor

See #654

When canceling a progress group, all monitors in the group should be canceled too.
fedejeanne added a commit to fedejeanne/eclipse.platform that referenced this issue Sep 7, 2023
@fedejeanne
Don't replace the monitor set by a group when starting a new job
1a29124 
- Only set the progress monitor of the Job if it hasn't one already.
This makes sure that the progress monitor set via Job::setProgressGroup
is not replaced with a new one and ergo when the group is canceled, all
the monitors in that group are canceled too.

- Add regression test:
JobTest.testSetProgressGroup_progressGroupIsCanceled_monitorInJobIsCanceled()

- Simplify private method JobManager::createMonitor(Job). Remove the
annotation "@GuardedBy" in it. Don't set the progress manager in
JobTest, it's unnecessary and it also lets the new regression test fail.

Fixes eclipse-platform#664
fedejeanne added a commit to fedejeanne/eclipse.platform that referenced this issue Sep 8, 2023
@fedejeanne
Don't replace the monitor set by a group when starting a new job
6cb32e0 
- Only set the progress monitor of the Job if it hasn't one already.
This makes sure that the progress monitor set via Job::setProgressGroup
is not replaced with a new one and ergo when the group is canceled, all
the monitors in that group are canceled too.

- Add regression test:
JobTest.testSetProgressGroup_progressGroupIsCanceled_monitorInJobIsCanceled()

- Simplify private method JobManager::createMonitor(Job). Remove the
annotation "@GuardedBy" in it. Don't set the progress manager in
JobTest, it's unnecessary and it also lets the new regression test fail.

Fixes eclipse-platform#664
@fedejeanne fedejeanne mentioned this issue Sep 8, 2023
Merged
fedejeanne added a commit to fedejeanne/eclipse.platform that referenced this issue Sep 8, 2023
@fedejeanne
Don't replace the monitor set by a group when starting a new job
c42251d 
- Only set the progress monitor of the Job if it hasn't one already.
This makes sure that the progress monitor set via Job::setProgressGroup
is not replaced with a new one and ergo when the group is canceled, all
the monitors in that group are canceled too.

- Add regression test:
JobTest.testSetProgressGroup_progressGroupIsCanceled_monitorInJobIsCanceled()

- Simplify private method JobManager::createMonitor(Job). Remove the
annotation "@GuardedBy" in it.

Fixes eclipse-platform#664
fedejeanne added a commit to fedejeanne/eclipse.platform that referenced this issue Sep 8, 2023
@fedejeanne
Don't replace the monitor set by a group when starting a new job
2e978fb 
- Only set the progress monitor of the Job if it hasn't one already.
This makes sure that the progress monitor set via Job::setProgressGroup
is not replaced with a new one and ergo when the group is canceled, all
the monitors in that group are canceled too.

- Add regression test:
JobTest.testSetProgressGroup_progressGroupIsCanceled_monitorInJobIsCanceled()

- Simplify private method JobManager::createMonitor(Job). Remove the
annotation "@GuardedBy" in it.

Fixes eclipse-platform#664
fedejeanne added a commit to fedejeanne/eclipse.platform that referenced this issue Sep 11, 2023
@fedejeanne
Don't replace the monitor set by a group when starting a new job
a055cfa 
- Only set the progress monitor of the Job if it hasn't one already.
This makes sure that the progress monitor set via Job::setProgressGroup
is not replaced with a new one and ergo when the group is canceled, all
the monitors in that group are canceled too.

- Add regression test:
JobTest.testSetProgressGroup_progressGroupIsCanceled_monitorInJobIsCanceled()

- Simplify private method JobManager::createMonitor(Job). Remove the
annotation "@GuardedBy" in it.

Fixes eclipse-platform#664
fedejeanne added a commit to fedejeanne/eclipse.platform that referenced this issue Sep 12, 2023
@fedejeanne
Don't replace the monitor set by a group when starting a new job
6ef011a 
- Only set the progress monitor of the Job if it hasn't one already.
This makes sure that the progress monitor set via Job::setProgressGroup
is not replaced with a new one and ergo when the group is canceled, all
the monitors in that group are canceled too.

- Add regression test:
JobTest.testSetProgressGroup_progressGroupIsCanceled_monitorInJobIsCanceled()

- Simplify private method JobManager::createMonitor(Job). Remove the
annotation "@GuardedBy" in it.

Fixes eclipse-platform#664
fedejeanne added a commit to fedejeanne/eclipse.platform that referenced this issue Sep 12, 2023
@fedejeanne
Don't replace the monitor set by a group when starting a new job
dcfaeeb 
- Only set the progress monitor of the Job if it hasn't one already.
This makes sure that the progress monitor set via Job::setProgressGroup
is not replaced with a new one and ergo when the group is canceled, all
the monitors in that group are canceled too.

- Add regression test:
JobTest.testSetProgressGroup_progressGroupIsCanceled_monitorInJobIsCanceled()

- Simplify private method JobManager::createMonitor(Job). Remove the
annotation "@GuardedBy" in it.

Fixes eclipse-platform#664
HeikoKlare pushed a commit that referenced this issue Sep 13, 2023
@fedejeanne @HeikoKlare
Don't replace the monitor set by a group when starting a new job
9c3525c 
- Only set the progress monitor of the Job if it hasn't one already.
This makes sure that the progress monitor set via Job::setProgressGroup
is not replaced with a new one and ergo when the group is canceled, all
the monitors in that group are canceled too.

- Add regression test:
JobTest.testSetProgressGroup_progressGroupIsCanceled_monitorInJobIsCanceled()

- Simplify private method JobManager::createMonitor(Job). Remove the
annotation "@GuardedBy" in it.

Fixes #664
fedejeanne added a commit to fedejeanne/eclipse.platform that referenced this issue Apr 4, 2024
@fedejeanne
Don't return null in JobManager::createMonitor
84578cc 
Revert unintentional change made in
9c3525c and don't return null when
creating a progress monitor. This is part of the contract of the method.

Contributes to
eclipse-platform#664
@fedejeanne fedejeanne mentioned this issue Apr 4, 2024
Merged
fedejeanne added a commit that referenced this issue Apr 4, 2024
@fedejeanne
Don't return null in JobManager::createMonitor
5919685 
Revert unintentional change made in
9c3525c and don't return null when
creating a progress monitor. This is part of the contract of the method.

Contributes to
#664
fedejeanne added a commit to fedejeanne/eclipse.platform that referenced this issue Apr 4, 2024
@fedejeanne
Don't return null in JobManager::createMonitor
1b99e42 
Revert unintentional change made in
9c3525c and don't return null when
creating a progress monitor. This is part of the contract of the method.

Contributes to
eclipse-platform#664
@fedejeanne fedejeanne mentioned this issue Apr 4, 2024
Closed
fedejeanne added a commit to fedejeanne/eclipse.platform that referenced this issue Apr 4, 2024
@fedejeanne
Revert automatic cleanups made in JobManager
e83e42c 
See eclipse-platform#1282 (comment)

Contributes to
eclipse-platform#664
@fedejeanne fedejeanne mentioned this issue Apr 4, 2024
Merged
fedejeanne added a commit to fedejeanne/eclipse.platform that referenced this issue Apr 4, 2024
@fedejeanne
Revert automatic cleanups made in JobManager
e4b5b9e 
Revert unintentional changes made in
5919685

See eclipse-platform#1282 (comment)

Contributes to
eclipse-platform#664
fedejeanne added a commit that referenced this issue Apr 4, 2024
@fedejeanne
Revert automatic cleanups made in JobManager
cccf0a0 
Revert unintentional changes made in
5919685

See #1282 (comment)

Contributes to
#664
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't replace the monitor set by a group when starting a new job fedejeanne/eclipse.platform
1 participant
@fedejeanne