-
Notifications
You must be signed in to change notification settings - Fork 665
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
JobQueue #8293
JobQueue #8293
Conversation
7c120a5
to
ced2bdf
Compare
a5defc3
to
ac3329a
Compare
ac3329a
to
aa9d6c6
Compare
This comment has been minimized.
This comment has been minimized.
7648160
to
b2d94cd
Compare
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.
Apart from the one comment I made (which is minor) I can not see anything suspicious.
Very good work. Makes the code more clear and solves problems 🥇
However, this is a significant change that needs good testing.
src/libsync/jobqueue.cpp
Outdated
} | ||
|
||
bool JobQueue::needsRetry(AbstractNetworkJob *job) const | ||
{ |
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 was wondering why this method is in JobQueue
and not in the Job
class directly. The job should be the source if retry is needed or not.
And the implementation indeed only uses job data to find out. So this method could be moved to Job::needsRetry()
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 thx
f47d264
to
8e8cf5b
Compare
Kudos, SonarCloud Quality Gate passed! |
No description provided.