You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardExpand all lines: CONTRIBUTING.md
+26-8Lines changed: 26 additions & 8 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -67,31 +67,50 @@ This section contains information and etiquette regarding pull requests (or "PRs
67
67
68
68
If you are creating a new component or significantly altering an existing one, please ensure that an issue has been created *before* you start any work, and that it has been assigned to you. The same goes for any big new additions or changes to the project structure, CI or documentation.
69
69
70
-
We want to avoid cases where developers build something that does not align with our wants & needs. We also want to be able to carefully plan our sprint and test cycles with minimal disruption. We especially want to avoid cases where two developers duplicate work.
70
+
We ask this because:
71
+
72
+
* we want to avoid cases where developers build something that does not align with our wants & needs
73
+
* we want to be able to carefully plan our sprint and test cycles with minimal disruption
74
+
* we want to avoid cases where two developers duplicate work
71
75
72
76
### Writing Code
73
77
74
78
The contents of a pull request should be related to a single issue only (or, at most, perhaps two very closely related issues). The reviewer has the right to reject a pull request that contains anything non-issue related.
75
79
76
-
Whilst it may be tempting to fix any other [broken windows](https://www.rtuin.nl/2012/08/software-development-and-the-broken-windows-theory/) that you encounter, it can distract the reviewer from the main issue at hand, make their job more time-consuming and increase the chance of regressions. Remember too, if the main issue were to get rolled back (for whatever reason), then there is a good chance that your non-related fixes would get rolled back along with it too. Which is probably not what anyone wants. So please be a good citizen and create separate issues or pull requests for the broken windows that you find.
80
+
Whilst it may be tempting to fix any [broken windows](https://www.rtuin.nl/2012/08/software-development-and-the-broken-windows-theory/) that you encounter, we ask you not to because:
81
+
82
+
* it can distract the reviewer from the main issue at hand
83
+
* it can add additional time needed for the reviewer
84
+
* it can increase the chance of regressions
85
+
* it can make rollbacks more difficult
86
+
87
+
So please be a good citizen and create separate issues or pull requests for any broken windows that you find.
77
88
78
89
### Moving Code
79
90
80
91
Please try and isolate "noisy" commits that only *move* many files or lines of code, from commits that actually modify code logic. The reviewer has the right to reject a pull request that is difficult to reason with due to too much *unnecessary* noise in the diff.
81
92
82
93
For example, assuming moving some code doesn't put the app into a broken state, the move can be considered a safe atomic commit (e.g. "moved functions x, y and z to bottom of file"). The actual *fixes* to functions x, y and z can then be made in a follow up commit (e.g. "fixed functions x, y and z") which will be easier for the reviewer to diff.
83
94
95
+
### Breaking Changes
96
+
97
+
Please think very carefully before introducing breaking API changes. Breaking changes may only be introduced in a major version. If you wish to alter the API *before* a major version, then aliases can often be a good way to achieve this. Using an alias, the old API can be deprecated and will still continue to function.
98
+
99
+
### Deprecating Code
100
+
101
+
Any deprecated code should be clearly documented in the code comments and the release notes. Deprecated code will typically be scheduled for removal in the next major version.
102
+
84
103
### Avoiding Conflicts
85
104
86
-
It is your responsibility to ensure that your feature branch has no merge conflicts with it's base branch. A pull request created and sent for review containing many conflicts does not inspire confidence in the reviewer.
105
+
It is your responsibility to ensure that your feature branch has no merge conflicts with its base branch. A pull request created and sent for review containing many conflicts does not inspire confidence in the reviewer.
87
106
88
-
The best way to ensure there are no conflicts is by keeping your branch up to date with it's base branch and resolving any conflicts in your own branch early & often. You may do this either by rebasing or merging. If you are the only developer working in the feature branch (you need to be sure), it is usually better to rebase. If another developer is sharing your branch, then merging is the safer option (as it doesn't require a force push).
107
+
The best way to ensure there are no conflicts is by keeping your branch up to date with its base branch and resolving any conflicts in your own branch early & often. If you are the only developer working in the feature branch, then it is usually better to rebase. If another developer is sharing your branch, then merging is the safer option (as it doesn't require a force push).
89
108
90
-
If you are working on a large feature, that takes many days, then there is a good chance that the base branch will have received many other commits during that time. If you wait till the very end you may find you have to deal with some difficult merge conflicts. On the other hand, if your pull request was created quickly and only touches a small surface area, then more than likely you will not run into any conflicts at the time the pull request is created. It doesn't hurt to keep a watchful eye on the base branch though.
109
+
If you are working on a large feature, that takes many days, then there is a good chance that the base branch will have received other commits during that time. If you wait till the very end before syncing your branch with the base branch, then you may have to deal with some difficult merge conflicts.
91
110
92
111
### Commit History
93
112
94
-
Whilst having multiple "work in progress", "almost done" and "merged branch" type commits in a feature branch is fine, we wish to refrain from them polluting the milestone and master branch history. On the flip side, although rarer in comparison, we might also have some *atomic* commits in our feature branch that we absolutely wish to keep as part of the project history.
113
+
Whilst having multiple "work in progress", "almost done" and "merged branch" type commits in a feature branch is fine, we wish to refrain from those commits polluting the milestone and master branch history. On the flip side, although rarer in comparison, we might also have some *atomic* commits in our feature branch that we absolutely wish to keep as part of the project history.
95
114
96
115
Before creating your pull request you have two options regarding what to do with the commit history of your feature branch:
97
116
@@ -177,8 +196,7 @@ A component is considered "done", and ready for merge into release branch, when
177
196
3) Code review
178
197
179
198
* Linter should catch syntax and style issues
180
-
* Prefer performance over readability
181
-
* Prefer readability over "space saving" code
199
+
* Prefer performance over readability, but readability over "space saving" code
182
200
* Apply the single-responsibility principle to functions
183
201
* Refactor long functions into several small functions
184
202
* Identify and move common utility functions to libraries or static methods
Copy file name to clipboardExpand all lines: README.md
+4Lines changed: 4 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -200,6 +200,10 @@ Given a version number MAJOR.MINOR.PATCH:
200
200
* MINOR version is incremented when we add functionality in a backwards-compatible manner
201
201
* PATCH version is incremented when we make backwards-compatible bug fixes.
202
202
203
+
### Deprecations
204
+
205
+
Deprecations will be communicated via [release notes](https://github.com/eBay/ebayui-core/releases), so please ensure that you read those carefully. In general, expect any deprecated feature to be removed in the next major version. However, in some cases we may wait a while longer.
206
+
203
207
### Issues
204
208
205
209
Please use our [issues page](https://github.com/eBay/ebayui-core/issues) to ask questions, report issues or submit feature requests.
0 commit comments