You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by GitBox <gi...@apache.org> on 2020/08/02 18:29:20 UTC

[GitHub] [systemds] j143 opened a new pull request #1004: Contributing guide inclusion of guidelines

j143 opened a new pull request #1004:
URL: https://github.com/apache/systemds/pull/1004


   **What this PR do?**
   1.  Provides more details license files.
   2. Add new section for general guidelines
   
   **TL;DR**
   Add SystemDS team's thinking/working philosophy based on the PR/commit/mailing list activity.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [systemds] mboehm7 commented on pull request #1004: Contributing guide inclusion of guidelines

Posted by GitBox <gi...@apache.org>.
mboehm7 commented on pull request #1004:
URL: https://github.com/apache/systemds/pull/1004#issuecomment-667715804


   Thanks @j143 - in general, it's is a great idea to add these guidelines. I just have to finish off some other things (review etc) and @Baunsgaard is on vacation, afterwards we come back to this PR and help bringing it in shape.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [systemds] Baunsgaard commented on a change in pull request #1004: Contributing guide inclusion of guidelines

Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on a change in pull request #1004:
URL: https://github.com/apache/systemds/pull/1004#discussion_r464899632



##########
File path: CONTRIBUTING.md
##########
@@ -24,8 +24,29 @@ Thanks for taking the time to contribute to SystemDS!
 The following are mostly guidelines, not rules. Use your best judgment, and feel free to propose changes to this document in a pull request.
 
 ___
-
-## Code Style
+### Contribution Guidelines and Standards
+
+Before contributing a pull request for [review](https://github.com/apache/systemds/pulls),
+let's make sure the changes are consistent with the guidelines and coding style.
+
+#### General Guidelines and Philosophy for contribution
+
+*   Inclusion of unit tests when contributing new features, will help
+    1. prove that the code works correctly, and
+    2. guard against future breaking changes.
+*   Bug fixes may or may not require unit tests, because the presence of bugs usually
+    indicates insufficient test coverage.

Review comment:
       This second comment is incorrect, not because of the first part before comma but because of the second part.
   I would like to remove this comment.

##########
File path: CONTRIBUTING.md
##########
@@ -35,26 +56,21 @@ We have provided at profile for java located in [Codestyle File ./docs/CodeStyle
 - [IntelliJ](https://imagej.net/Eclipse_code_style_profiles_and_IntelliJ)
 - [Visual Studio Code](https://stackoverflow.com/questions/46030629/need-to-import-eclipse-java-formatter-profile-in-visual-studio-code)
 
-## License
+#### License
 
-Each file in the project has a header license, when adding files remember to add an appropriate comment for that filetype containing the license. One Example is this file: [RAW Link](https://raw.githubusercontent.com/apache/systemds/master/CONTRIBUTING.md). The specific type of commenting `\** *\` or `# ...` varies depending on filetype, but the content is the same.
+Including a license at the top of new files helps validate the consistency of license.
 
-```code
-Licensed to the Apache Software Foundation (ASF) under one or more
-contributor license agreements.  See the NOTICE file distributed with
-this work for additional information regarding copyright ownership.
-The ASF licenses this file to you under the Apache License, Version 2.0
-(the "License"); you may not use this file except in compliance with
-the License.  You may obtain a copy of the License at
+Examples:
 
-http://www.apache.org/licenses/LICENSE-2.0
+- [C/C++/cmake/cuda](./src/main/cpp/libmatrixdnn.h#L1-L18)
+- [Python](./src/main/python/create_python_dist.py#L1-L21)
+- [Java](./src/main/java/org/apache/sysds/api/ConfigurableAPI.java#L1-L18)
+- [Bash](./src/main/bash/sparkDML2.sh#L2-L21)
+- [XML/HTML](./src/assembly/bin.xml#L2-L19)
+- [dml/R](./scripts/algorithms/ALS-CG.dml#L1-L20)
+- [Makefile/.proto](./src/main/cpp/kernels/Makefile#L1-L18)
+- Markdown - refer to the top of this file!

Review comment:
       I really like this change!

##########
File path: CONTRIBUTING.md
##########
@@ -24,8 +24,29 @@ Thanks for taking the time to contribute to SystemDS!
 The following are mostly guidelines, not rules. Use your best judgment, and feel free to propose changes to this document in a pull request.
 
 ___
-
-## Code Style
+### Contribution Guidelines and Standards
+
+Before contributing a pull request for [review](https://github.com/apache/systemds/pulls),
+let's make sure the changes are consistent with the guidelines and coding style.
+
+#### General Guidelines and Philosophy for contribution
+
+*   Inclusion of unit tests when contributing new features, will help
+    1. prove that the code works correctly, and
+    2. guard against future breaking changes.
+*   Bug fixes may or may not require unit tests, because the presence of bugs usually
+    indicates insufficient test coverage.
+*   Formatting changes can be handled in a separate PR.
+    Example [`bf4ba16b`](https://github.com/apache/systemds/commit/bf4ba16b9aaa9afee20a3f1c03b0ff49c5346a9d)
+*   New features (e.g., a new cutting edge machine learning algorithm) typically will
+    live in [scripts/staging](./scripts/staging) or its equivalent folder for specific
+    feature to get some airtime and sufficient testing before a decision is made regarding
+    whether they are to migrated to the top-level.
+*   When a new contribution is made to SystemDS, the maintenance burden is (by default)
+    transferred to the SystemDS team. The benefit of the contribution is to be compared
+    against the cost of maintaining the feature.

Review comment:
       This means make maintainability a high priority!




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [systemds] mboehm7 edited a comment on pull request #1004: Contributing guide inclusion of guidelines

Posted by GitBox <gi...@apache.org>.
mboehm7 edited a comment on pull request #1004:
URL: https://github.com/apache/systemds/pull/1004#issuecomment-667715804






----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [systemds] j143 merged pull request #1004: Contributing guide inclusion of guidelines

Posted by GitBox <gi...@apache.org>.
j143 merged pull request #1004:
URL: https://github.com/apache/systemds/pull/1004


   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [systemds] j143 commented on pull request #1004: Contributing guide inclusion of guidelines

Posted by GitBox <gi...@apache.org>.
j143 commented on pull request #1004:
URL: https://github.com/apache/systemds/pull/1004#issuecomment-667708855


   cc @mboehm7 @Baunsgaard 


----------------------------------------------------------------
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:
users@infra.apache.org