cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bhaisaab <...@git.apache.org>
Subject [GitHub] cloudstack pull request: systemvm: preserve file permissions, set ...
Date Tue, 19 Apr 2016 17:38:06 GMT
Github user bhaisaab commented on the pull request:

    https://github.com/apache/cloudstack/pull/1420#issuecomment-212034630
  
    @swill we may ignore them, those are non-issues unless @rafaelweingartner has strong opinions
    
    @rafaelweingartner things you can do to improve your reviews:
    - Google things when you don't understand them, try to read documentation for tool/domain
specific things
    - Try to read CloudStack's code (and not just the diffs) and try to see the bigger picture,
understand the why behind the PR and ask questions on dev@ when you hit dead-end. Everyone
wants to help each other but we should avoid wasting time as much as we can, esr has documented
this nicely here: http://www.catb.org/esr/faqs/smart-questions.html
    - Stackoverflow helps as well (see this for example, http://stackoverflow.com/questions/13268796/umask-difference-between-0022-and-022)
    - Understand that adding non-issue comment adds frictions on PRs, while pragmatic reviews
(especially those around design, security, real world usage, upgrades and bigger picture)
helps a ton
    - The PR author may not be responsible or required to explain to each individual how each
part of the system (while they normally would, but we should not expect that) especially loosely
related technologies including dependencies work (for example for this PR, I'm not required
to explain you how Linux file permissions or systemd works -- if you don't know that you may
invest time in learning them on your own)
    - Avoid leading a PR and their author to change for a system-wide refactoring: while in
general the codebase is inconsistent due to lack of coding guidelines and lack of historic
enforcement, your specific taste of how variables are named, what utilities to use, how to
write comments, how to place files and structure packages may differ from others and other
such cosmetic preferences. All of such things if necessary to fix should have a separate PR/effort
and not to be coupled with an given PR.
    - Other than checkstyle maven plugin we don't have any coding enforcement, if you've strong
opinions and want to see change -- you may also consider taking a bigger effort to define
a coding guideline and have contributors adopt that over time.
    - In the community, we value people over code -- therefore, avoid idealism and favour
pragmatism else you risk adding unnecessary friction because we'll in general block PRs until
an outstanding issue is resolved


---
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