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

Fix incorrect state after indicator cancellation #346

Open
wants to merge 2 commits into
base: development
Choose a base branch
from
Open

Fix incorrect state after indicator cancellation #346

wants to merge 2 commits into from
+21 −4

Conversation

arksap2002
Copy link
Collaborator

@arksap2002 arksap2002 commented Sep 3, 2024 •
edited
Loading

Description of changes made

isProcessStopped method calls relocation due to long time methods processing

Other notes

Closes #213 #314

  • I have checked that I am merging into correct branch

@arksap2002 arksap2002 added bug Something isn't working Ready for review PR redy for review labels Sep 3, 2024
@arksap2002 arksap2002 changed the title fix incorrect state after indicator cancellation Fix incorrect state after indicator cancellation Sep 3, 2024
@arksap2002 arksap2002 added Ready for review PR redy for review and removed Ready for review PR redy for review labels Sep 3, 2024
@arksap2002
Copy link
Collaborator Author

Similar to #216

pderakhshanfar
pderakhshanfar approved these changes Sep 18, 2024
View reviewed changes
Copy link
Collaborator

@pderakhshanfar pderakhshanfar left a comment

Choose a reason for hiding this comment

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

For now, it is fine, but in the future, we need to find a better way to handle cancelation instead of repeating isCanceled check after each statement

@pderakhshanfar pderakhshanfar added the In progress PR is in progress label Sep 18, 2024
@arksap2002 arksap2002 linked an issue Sep 18, 2024 that may be closed by this pull request
stop() should be called only if start() called before #314
Open
@Vladislav0Art
Copy link
Collaborator

How do you ensure that #314 gets closed?
@arksap2002

@Vladislav0Art
Copy link
Collaborator

For now, it is fine, but in the future, we need to find a better way to handle cancelation instead of repeating isCanceled check after each statement

Kotlin coroutines support some smart cancelation mechanisms; transitioning to them in the future should help with this.

Vladislav0Art
Vladislav0Art approved these changes Oct 2, 2024
View reviewed changes
Copy link
Collaborator

@Vladislav0Art Vladislav0Art left a comment

Choose a reason for hiding this comment

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

Before merging the PR, please answer this question.
@arksap2002

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pderakhshanfar pderakhshanfar pderakhshanfar approved these changes

@Vladislav0Art Vladislav0Art Vladislav0Art approved these changes

Assignees
No one assigned
Labels
bug Something isn't working In progress PR is in progress Ready for review PR redy for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

stop() should be called only if start() called before Incorrect state after indicator cancellation
3 participants
@arksap2002 @Vladislav0Art @pderakhshanfar