Cloudstack 8656: do away with more silently ignoring exceptions.#654
Conversation
There was a problem hiding this comment.
Just curious why we went from protected to public here?
cc74f80 to
e44eea7
Compare
|
cloudstack-pull-rats #194 FAILURE |
|
cloudstack-pull-requests #891 ABORTED |
|
cloudstack-pull-rats #195 ABORTED |
|
cloudstack-pull-analysis #127 UNSTABLE |
|
cloudstack-pull-analysis #128 ABORTED |
|
cloudstack-pull-requests #892 ABORTED |
moved closeable util function up the hierarchy
e44eea7 to
e8a00ed
Compare
|
cloudstack-pull-requests #898 FAILURE |
|
cloudstack-pull-rats #201 ABORTED |
|
cloudstack-pull-analysis #134 UNSTABLE |
like previous in hyperv now in vmware
but log it if it does anyway
why ignore the exception and then return false anyway?
|
cloudstack-pull-rats #203 ABORTED |
|
cloudstack-pull-requests #900 ABORTED |
|
cloudstack-pull-requests #902 FAILURE |
|
cloudstack-pull-analysis #136 SUCCESS |
|
cloudstack-pull-rats #306 SUCCESS |
|
cloudstack-pull-requests #1002 SUCCESS |
|
cloudstack-pull-analysis #234 SUCCESS |
|
cloudstack-pull-rats #307 SUCCESS |
|
cloudstack-pull-requests #1003 SUCCESS |
|
cloudstack-pull-analysis #238 SUCCESS |
|
cloudstack-pull-analysis #239 SUCCESS |
|
cloudstack-pull-rats #308 SUCCESS |
|
cloudstack-pull-requests #1004 SUCCESS |
|
LGTM |
|
look alright, but could it introduce any regressions in the upgrade path? Also, I would love to see a single squashed patched with the changes given ~30 patches for |
|
All kinds of regressions may be introduced. This is (one of the reasons) why I will not squash. for a single regression we only have to revert a single commit, for the bunch we will have to revert the merge commit. |
|
Another way of fixing a regression could be to simply fix the issue, instead of reverting at all. |
|
cloudstack-pull-analysis #240 SUCCESS |
|
You know I am a great advocate of not squashing, right. All commits are atomic. Yes there is a lot because the work has also been scattered and a lot. |
|
@DaanHoogland I agree, we should only squash if there is no value to have the commits separated. I usually ask for squashing when feedback was processed in a separate commit (say remove a comment or fix a small thing). That you want squashed so it becomes an atomic commit. If commits are already atomic, I'm OK with not squashing. It's your call here ;-) LGTM from me. |
|
@bhaisaab Indeed, let's fix forward when needed :-) There's always a risk, but I'm happy to see this work being done 👍 |
Cloudstack 8656: do away with more silently ignoring exceptions.a lot of messages added. some restructuring for test exception assertions and try-with-resource blocks * pr/654: (29 commits) CLOUDSTACK-8656: more logging instead of sysout CLOUDSTACK-8656: use catch block for validation CLOUDSTACK-8656: class in json specified not found CLOUDSTACK-8656: removed unused classes CLOUDSTACK-8656: restructure of tests CLOUDSTACK-8656: reorganise sychronized block CLOUDSTACK-8656: restructure tests to ensure exception throwing CLOUDSTACK-8656: validate the throwing of ServerApiException CLOUDSTACK-8656: logging ignored exceptions CLOUDSTACK-8656: try-w-r removes need for empty catch block CLOUDSTACK-8656: try-w-r instead of clunckey close-except CLOUDSTACK-8656: deal with empty SQLException catch block by try-w-r CLOUDSTACK-8656: unnecessary close construct removed CLOUDSTACK-8656: message about timed buffer logging CLOUDSTACK-8656: message about invalid number from store CLOUDSTACK-8656: move cli test tool to separate file CLOUDSTACK-8656: exception is the rule for some tests CLOUDSTACK-8656: network related exception logging CLOUDSTACK-8656: reporting ignored exceptions in server CLOUDSTACK-8656: log in case we are on a platform not supporting UTF8 ... Signed-off-by: Remi Bergsma <github@remi.nl>
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
a lot of messages added.
some restructuring for test exception assertions and try-with-resource blocks