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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
