-
Notifications
You must be signed in to change notification settings - Fork 23
Added the proper logging workflow in upgrade #382
Added the proper logging workflow in upgrade #382
Conversation
0800eee
to
aca56d3
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.
Looks good 👍
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.
Thanks for taking this up @devendra104, its long pending improvement.
Btw loggings need to be more precise and clear on what entity the change is happeing / going to happen. The names are better than ids and clear. If you want ids for debugging purpose keep both name and id in logging.
Also pre logging before major happenings like publish, sync is very welcome as a person get an idea on whats happening untill the success message.
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.
Thanks for taking this up @devendra104, its long pending improvement.
Btw loggings need to be more precise and clear on what entity the change is happeing / going to happen. The names are better than ids and clear. If you want ids for debugging purpose keep both name and id in logging.
Also pre logging before major happenings like publish, sync is very welcome as a person get an idea on whats happening untill the success message.
aca56d3
to
395582f
Compare
Thanks @jyejare for providing the valuable comment. |
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.
ACK pending few nitpicks
395582f
to
c495674
Compare
c495674
to
875f3c1
Compare
#379