Skip to content

Conversation

@firewave
Copy link
Collaborator

@firewave firewave commented Jul 26, 2023

As detailed in #4985 (comment) it doesn't provide much progress and adds unnecessary complexity to the ErrorLogger and CppcheckExecutor.

We should probably remove TokenImpl::mProgressValue as well.

@firewave
Copy link
Collaborator Author

As a fix for the democlient it should possibly perform a delayed call to Settings::Terminate() instead which should provide a similar behavior.

I am also open to keeping the option and nop'ing it for now and provide a much better progress implementation later on.

@firewave
Copy link
Collaborator Author

This approach goes probably a bit too far. I will revise the changes.

@danmar
Copy link
Owner

danmar commented Aug 7, 2023

as I said.. we have reasons to keep this. maybe you can consider using the reportProgress instead.

@firewave
Copy link
Collaborator Author

firewave commented Aug 7, 2023

Yes, this goes in hand with #5288 and https://trac.cppcheck.net/ticket/11248 as it is about progress indication as well as providing interruption points for cancellation (especially if not using threads instead of processes).

Maybe this could even be addressed with the current executor rework I am doing.

@firewave
Copy link
Collaborator Author

firewave commented Aug 7, 2023

I will file a ticket about this in the next few days.

@firewave
Copy link
Collaborator Author

Seems there already was a ticket about this: https://trac.cppcheck.net/ticket/3450. I also came to some of the same conclusions before even reading it.

@firewave
Copy link
Collaborator Author

Closing as this was made obsolete by #5658 and #5353.

@firewave firewave closed this Sep 16, 2024
@firewave firewave deleted the reportprogress branch September 16, 2024 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants