rafaelweingartner commented on a change in pull request #2776: Issue 2774:
Changed the implementation of isVolumeOnManagedStorage(VolumeInfo) to…
URL: https://github.com/apache/cloudstack/pull/2776#discussion_r207190116
##########
File path:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
##########
@@ -196,10 +196,16 @@ public StrategyPriority canHandle(DataObject srcData,
DataObject destData) {
}
private boolean isVolumeOnManagedStorage(VolumeInfo volumeInfo) {
Review comment:
No, we have never discussed the need for unit tests. However, CloudStack has
been in Apache for 5/6/7 (I do not know the number) years now, and sometimes we
still see bad (really bad) practices around, which only hurts us in the long
run and make the code maintenance harder and harder.
Don’t get me wrong, from 2013 when I started working with CloudStack until
now, the code base has improved a lot (really a lot). I only mentioned this
topic here because I know Daan and I do like to hear his opinions.
BTW: that is the topic I am addressing with my talk in ApacheCon.
Thanks for the test cases for the `canHandle` method @DaanHoogland.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
