diff --git a/Contributing_README.md b/Contributing_README.md index 5c1970a..a96bc38 100644 --- a/Contributing_README.md +++ b/Contributing_README.md @@ -54,14 +54,14 @@ A list of acceptance criteria (What is needed for the issue to be considered com - All code files in this organization - relating to the ExAlta3 mission and onward - must follow a particular coding format and style. An automatic workflow action will test your code against theses standards automatically upon any request to merge with main. - For Python: It is suggested to install a pylint linter extension in your IDE, to write code that follows these standards as you go. - For C/C++: It is suggested to intsall a clang linter extension in your IDE, to write code that follows these standards as you go. - -### Copying code +- For Rust: It is suggested to use cargo clippy to lint your code, additionally use the rust analyzer (available as extension in your IDE) to write code that follows these standards as you go. +### Copying code - Be sure to included referrences to code that is not your original work in order to prevent plagarism. Referencing code may also help members find information that may be useful to them. ### Copyright - Ensure that every source file written has associated 'author' and 'copyright' metadata included in the file (for now it is found in the bottom, two lines after all code). - Because these items will exist in every source file, style checkers like pylint must be told explicitly told to ignore them otherwise your code will fail style tests -- The following is an example template for the copyright to be added to ALL source files written by contributing members: +- The following is an example template for the copyright to be added to ALL source files written by contributing members: #### Copyright template Copyright 2023 [Your name here] @@ -74,4 +74,4 @@ A list of acceptance criteria (What is needed for the issue to be considered com distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and - limitations under the License. \ No newline at end of file + limitations under the License. diff --git a/Pull_Requests.md b/Pull_Requests.md new file mode 100644 index 0000000..afa1c0c --- /dev/null +++ b/Pull_Requests.md @@ -0,0 +1,47 @@ +# Code Reviews + +## What is a code review? +When you, or another team member wants add changes to an AlbertaSat repository they will go through a process defined by the (contributing guideline section)[Contributing_README.md]. This typically involves creating a branch, making changes, making a pull request, code review, then the reviewer either asks for changes to be made or approves the pull request. After the pull request is approved the code can be merged to main, or additional changes can be made. In this documentation we will go over guidelines for reviewing and writing pull requests. + +## How should a pull request look like? +__This section we will go over to how write a proper pull request that allows code reviewers to quickly and efficiently review code.__\ +\ +The following are some general rules of what a pull request should look like:\ +- Change the default pull request name from the branch name to a descriptive name summarizes the changes you want to make. +- Add a useful description that includes the following: + - The issue number linked to the pull request. + - Any dependent pull requests in a different repository that should be reviewed first. + - Describe the changes you made and how the changes you made solve the root issue. + - If neccesary, clearly state any feedback you are looking for with a mention to the person who can help with feedback. +- If your pull request is still a work in progress, prefix the name of the pull request with "[WIP]". +- Convert your PR to a draft if you are not ready to have it reviewed, you can always convert it back to a PR at a later time. + +## How should a pull request be reviewed? +__This section we will go over to how review a pull request.__\ +\ +The following are some general rules of how a reviewer should review a pull request:\ +- If the PR is labelled as "WIP" then it should not be approved. +- Thoroughly go over all changes made and if neccesary leave feedback. +- When leaving feed back try to be as specific as possible (Comment with the specific code line, and the requested change). +- Checkout the branch on your machine and test to ensure it does not break anything and implements the feature as intended. +- When requesting changes, be specifc about what you want changed (comment or propose a modified code block or line). +- Adhere to definitions of pull request approval, request for changes, and comments found in definition section. +- After the pull request has been merged (by either reviewer or creator of the pull request) the corresponding branch should be deleted and the pull request should be closed. + +## Definitions + +__Approve__: A pull request should only be approved after the reviewer has completed the following: +- reviewer has reviewed all code +- reviewer has tested code by checking out branch +- reviewer has confirmed the pull request implements the intended feature +- the pull request is not marked as in progress +- the pull request does not break any existing features +- creator of pull request has confirmed it is ready to be merged\ +\ +In summation, the pull request can only be marked as approved once it is ready to be merged with main. If any changes or feedback is given, the changes should be implemented and the creator of the pull request should re-request a review. + +__Request for Changes__: A pull request can be reviewed and the reviewer can request changes, all changes should be resolved (fixed or ruled as not neccesary) before being re-reviewed. The reviewer should do their best to be specific about what changes should be made and if neccesary, explain how to implement them. + + +__Comment__: A pull request can be reviewed by leaving a comment which is general feedback for the creator can implement. After the creator of the pull request has resolved the feed back they can re-request a review. Although the feed back is general, the reviewer should try and be as helpful as possible in their feedback to help resolve the issue. +