-
Notifications
You must be signed in to change notification settings - Fork 10
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
Change reporting calls timings to ensure consistency #226
base: development
Are you sure you want to change the base?
Change reporting calls timings to ensure consistency #226
Conversation
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 think we should solve this with an interface change which allows for the calling time to be added to the reporting time. So even when reporting would take 10 seconds the original time is used. If an endstep report fails we remain with the wait group with the proposed change
f3b4a7b
to
49c1571
Compare
Sigrid maintainability feedbackSigrid compared your code against the baseline of 2024-10-14. 👍 What went well?You fixed or improved 0 refactoring candidates. 👎 What could be better?Unfortunately, 24 refactoring candidates were introduced or got worse.
📚 Remaining technical debt9 refactoring candidates didn't get better or worse, but are still present in the code you touched.
Sigrid ratings
Did you find this feedback helpful?We would like to know your thoughts to make Sigrid better. |
Why is sigrid bullying me tho |
Added a sync.WaitGroup to the reporter module, which is used to ensure that the routine that reports the workflow end wg.Wait()-s before executing, allowing for all other reporting subroutines to record the progress correctly.
Note: I don't know if this can be unit-tested.