cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rafaelweingartner <...@git.apache.org>
Subject [GitHub] cloudstack pull request: CLOUDSTACK-8805: Domains become inactive ...
Date Wed, 16 Sep 2015 22:44:54 GMT
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.
---

Mime
View raw message