Ethanlm commented on a change in pull request #3092: STORM-3474 Large fragmented cluster scheduling time test URL: https://github.com/apache/storm/pull/3092#discussion_r309312828
########## File path: storm-server/src/main/java/org/apache/storm/scheduler/resource/strategies/scheduling/ConstraintSolverStrategy.java ########## @@ -41,6 +41,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +// When making changes to scheduling strategy, be sure to run performance performance unit test +// TestResourceAwareScheduler::testLargeFragmentedClusterScheduling Review comment: I don't think this is a good practice to link to a unit test. And we don't need to worry about reminding developers to run a specific unit test because running unit tests is something that will just happen. Developers will run all the related unit tests or at least let Travis run the unit tests. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
