Skip to content

Commit 938076e

Browse files
Merge pull request #1458 from allmightyspiff/issues1457
updated contributing guide
2 parents 7927049 + 262507e commit 938076e

File tree

1 file changed

+57
-5
lines changed

1 file changed

+57
-5
lines changed

Diff for: CONTRIBUTING.md

+57-5
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,22 @@
33
We are happy to accept contributions to softlayer-python. Please follow the
44
guidelines below.
55

6-
* Sign our contributor agreement (CLA) You can find the [CLA here](./docs/dev/cla-individual.md).
6+
## Procedural
77

8-
* If you're contributing on behalf of your employer we'll need a signed copy of our corporate contributor agreement (CCLA) as well. You can find the [CCLA here](./docs/dev/cla-corporate.md).
9-
10-
* Fork the repo, make your changes, and open a pull request.
8+
1. All code changes require a corresponding issue. [Open an issue here](https://github.com/softlayer/softlayer-python/issues).
9+
2. Fork the [softlayer-python](https://github.com/softlayer/softlayer-python) repository.
10+
3. Make any changes required, commit messages should reference the issue number (include #1234 if the message if your issue is number 1234 for example).
11+
4. Make a pull request from your fork/branch to origin/master
12+
5. Requires 1 approval for merging
1113

1214
* Additional infomration can be found in our [contribution guide](http://softlayer-python.readthedocs.org/en/latest/dev/index.html)
1315

16+
## Legal
17+
18+
* See our [Contributor License Agreement](./docs/dev/cla-individual.md). Opening a pull request is acceptance of this agreement.
19+
20+
* If you're contributing on behalf of your employer we'll need a signed copy of our corporate contributor agreement (CCLA) as well. You can find the [CCLA here](./docs/dev/cla-corporate.md).
21+
1422

1523
## Code style
1624

@@ -101,4 +109,48 @@ order_args = getattr(order_call[0], 'args')[0]
101109

102110
# Test our specific argument value
103111
self.assertEqual(123, order_args['hostId'])
104-
```
112+
```
113+
114+
115+
## Project Management
116+
117+
### Issues
118+
119+
* ~~Title~~: Should contain quick highlight of the issue is about
120+
* ~~Body~~: All the technical information goes here
121+
* ~~Assignee~~: Should be the person who is actively working on an issue.
122+
* ~~Label~~: All issues should have at least 1 Label.
123+
* ~~Projects~~: Should be added to the quarerly Softlayer project when being worked on
124+
* ~~Milestones~~: Not really used, can be left blank
125+
* ~~Linked Pull Request~~: Should be linked to the relavent pull request when it is opened.
126+
127+
### Pull Requests
128+
129+
* ~~Title~~: Should be similar to the title of the issue it is fixing, or otherwise descibe what is chaning in the pull request
130+
* ~~Body~~: Should have "Fixes #1234" at least, with some notes about the specific pull request if needed. Most technical information should still be in the github issue.
131+
* ~~Reviewers~~: 1 Reviewer is required
132+
* ~~Assignee~~: Should be the person who opened the pull request
133+
* ~~Labels~~: Should match the issue
134+
* ~~Projects~~: Should match the issue
135+
* ~~Milestones~~: Not really used, can be left blank
136+
* ~~Linked issues~~: If you put "Fixes #<Issue number>" in the body, this should be automatically filled in, otherwise link manually.
137+
138+
### Code Reviews
139+
All issues should be reviewed by at least 1 member of the SLDN team that is not the person opening the pull request. Time permitting, all members of the SLDN team should review the request.
140+
141+
#### Things to look for while doing a review
142+
143+
As a reviewer, these are some guidelines when doing a review, but not hard rules.
144+
145+
* Code Style: Generally `tox -e analysis` will pick up most style violations, but anything that is wildly different from the normal code patters in this project should be changed to match, unless there is a strong reason to not do so.
146+
* API Calls: Close attention should be made to any new API calls, to make sure they will work as expected, and errors are handled if needed.
147+
* DocBlock comments: CLI and manager methods need to be documented well enough for users to easily understand what they do.
148+
* Easy to read code: Code should generally be easy enough to understand at a glance. Confusing code is a sign that it should either be better documented, or refactored a bit to be clearer in design.
149+
150+
151+
### Testing
152+
153+
When doing testing of a code change, indicate this with a comment on the pull request like
154+
155+
:heavy_check: `slcli vs list --new-feature`
156+
:x: `slcli vs list --broken-feature`

0 commit comments

Comments
 (0)