Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
From: rafaelweingartner <git@git.apache.org>
To: dev@cloudstack.apache.org
Reply-To: dev@cloudstack.apache.org
References: <git-pr-775-cloudstack@git.apache.org>
In-Reply-To: <git-pr-775-cloudstack@git.apache.org>
Subject: [GitHub] cloudstack pull request: CLOUDSTACK-8805: Domains become
 inactive ...
Content-Type: text/plain
Message-Id: <20150916224454.37288E00AB@git1-us-west.apache.org>
Date: Wed, 16 Sep 2015 22:44:54 +0000 (UTC)

Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/775#issuecomment-140915157
  
    @ DaanHoogland, I understand you, I just commented that to show to @nitin-maharana that “we” (I) understand that writing a test case to a method like that can be pretty hard, and that we are open to discuss and help with that. 
    
    I just wanted to show support, because I have seen in other open source projects people rejecting PRs, just because the guy that created the PR is not well-known in the community and he/she had not written test cases for huge methods in which the developer had just made a pretty small fix like @nitin-maharana is doing. Most of the time developers debug the code, looking for the crux of the problem he/she is having, find and fix it without knowing the whole idea of the method, especially when methods have generic names, perform a lot of tasks and do not have proper java docs.
    
    For instance: when I read, “cleanupDomain”, I do not fully understand what the method is doing. Is it a periodical task that is intended to remove dirty from domains? Is it meant to remove resources that are no longer used by users of a domain?
    
    I know that one of the tasks of that method is to remove/disable domains and their sub-domains, but to disable/remove something for me is different from cleaning something up.
    
    Every time I review a code and see a clear opportunity to extract code and write test cases I suggest that, but when that does not happens, I do not feel comfortable asking for them and blocking a PR. That might not feel welcoming for a new contributor that is devoting some of his/her time for free in a project.
    
    Bottom line is that we liked the job he has done and are here to help him with ideas in how to write at least one test case for the change he has made.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---