Skip to content

Commit 11656e4

Browse files
committed
revert
1 parent f6b6923 commit 11656e4

File tree

4 files changed

+21932
-160
lines changed

4 files changed

+21932
-160
lines changed

CONTRIBUTING.md

Lines changed: 153 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,14 @@
22

33
## Table of Contents <!-- omit in toc -->
44
- [Contributing](#contributing)
5+
- [Issue vs. Pull Request](#issue-vs-pull-request)
6+
- [Templates](#templates)
57
- [Why Contributing?](#why-contributing)
8+
- [Contribution FAQs](#contribution-faqs)
9+
- [Reviewer Role](#reviewer-role)
10+
- [Review Feedback](#review-feedback)
11+
- [Merge Readiness](#merge-readiness)
12+
- [Review Validity](#review-validity)
613
- [Environment Setup](#environment-setup)
714
- [Recommended Tools](#recommended-tools)
815
- [Setting up your local machine](#setting-up-your-local-machine)
@@ -20,11 +27,19 @@
2027
- [Parse Error](#parse-error)
2128
- [Parse Server Configuration](#parse-server-configuration)
2229
- [Pull Request](#pull-request)
30+
- [Commit Message](#commit-message)
2331
- [Breaking Change](#breaking-change)
2432
- [Merging](#merging)
2533
- [Breaking Change](#breaking-change-1)
2634
- [Reverting](#reverting)
35+
- [Security Vulnerability](#security-vulnerability)
36+
- [Releasing](#releasing)
37+
- [General Considerations](#general-considerations)
2738
- [Major Release / Long-Term-Support](#major-release--long-term-support)
39+
- [Preparing Release](#preparing-release)
40+
- [Publishing Release (forward-merge):](#publishing-release-forward-merge)
41+
- [Publishing Hotfix (back-merge):](#publishing-hotfix-back-merge)
42+
- [Publishing Major Release (Yearly Release)](#publishing-major-release-yearly-release)
2843
- [Versioning](#versioning)
2944
- [Code of Conduct](#code-of-conduct)
3045

@@ -42,6 +57,22 @@ When you are ready to code, you can find more information about opening a pull r
4257

4358
Whether this is your first contribution or you are already an experienced contributor, the Parse Community has your back – don't hesitate to ask for help!
4459

60+
### Issue vs. Pull Request
61+
62+
An issue is required to be linked in every pull request. We understand that no-one likes to create an issue for something that appears to be a simple pull request, but here is why this is beneficial for everyone:
63+
64+
- An issue get more visibility than a pull request as issues can be pinned, receive bounties and it is primarily the issue list that people browse through rather than the more technical pull request list. Visibility is a key aspect so others can weigh in on issues and contribute their opinion.
65+
- The discussion in the issue is different from the discussion in the pull request. The issue discussion is focused on the issue and how to address it, whereas the discussion in the pull request is focused on a specific implemention. An issue may even have multiple pull requests because either the issue requires multiple implementations or multiple pull requests are opened to compare and test different approaches to later decide for one.
66+
- High-level conceptual discussions about the issue should be still available, even if a pull request is closed because its appraoch was discarded. If these discussions are in the pull request instead, they can easily become fragmented over multiple pull requests and issues, which can make it very hard to make sense of all aspects of an issue.
67+
68+
### Templates
69+
70+
You are required to use and completely fill out the templates for new issues and pull requests. We understand that no-one enjoys filling out forms, but here is why this is beneficial for everyone:
71+
72+
- It may take you 30 seconds longer, but will save even more time for everyone else trying to understand your issue.
73+
- It helps to fix issues and merge pull requests faster as reviewers spend less time trying to understand your issue.
74+
- It makes investigations easier when others try to understand your issue and code changes made even years later.
75+
4576
## Why Contributing?
4677

4778
Buy cheap, buy twice. What? No, this is not the Economics 101 class, but the same is true for contributing.
@@ -63,6 +94,46 @@ Consider the benefits you get:
6394

6495
Most importantly, with every contribution you improve your skills so that future contributions take even less time and you get all the benefits above for free — easy choice, right?
6596

97+
## Contribution FAQs
98+
99+
### Reviewer Role
100+
101+
> *Instead of writing review comments back-and-forth, why doesn't the reviewer just write the code themselves?*
102+
103+
A reviewer is already helping you to make a code contribution through their review. A reviewer *may* even help you to write code by actually writing it for you, but is not obliged to do so.
104+
105+
GitHub allows reviewers to suggest and write code changes as part of the review feedback. These code suggestions are likely to contain mistakes due to the lack of code syntax checks when writing code directly on GitHub. You should therefore always review these suggestions before accepting them, ideally in an IDE. If you merge a code suggestion and the CI then fails, take another look at the code change before asking the reviewer for help.
106+
107+
### Review Feedback
108+
109+
> *It takes too much effort to incorporate the review feedback, why why can't you just merge my pull request?*
110+
111+
If you are a new contributor, it's naturally a learning experience for you and therefore takes longer. We welcome contributors of any experience levels and gladly support you in getting familiar with the code base and our quality standards and contribution requirements. In return we expect you to be open to and appreciative of the reviewers' feedback.
112+
113+
In a large pull request, it can be a significant effort to bring it over the finish line. Luckily this is a collaborative environment and others are free to jump in to contribute to the pull request to share the effort. You can either give others access to your fork or they can open their own pull request based on your previous work.
114+
115+
If you are out of resources stay calm, explain your personal constraints (expertise or time) and ask for help. Wasting time by complaining about the amount of review comments will neither use your own time in a meaningful way, nor the time of others who read your complaint.
116+
117+
This is a collaborative enviroment in which everyone works on a common goal - to get a pull request ready for merging. Reviewers are working *with* you to get your pull request ready, *not against you*.
118+
119+
**❗️ Always be mindful that the reviewers' efforts are an integral part of code contribution. Their review is as important as your written code and their review time is a valuable as your coding time.**
120+
121+
### Merge Readiness
122+
123+
> *The feature already works, why do you request more changes instead of just merging my pull request?*
124+
125+
A feature may work for your own use case or in your own environment, but that doesn't necessarily mean that it's ready for merging. Aside from code quality and code style requirements, reviewers also review based on strategic and architectural considerations. It's often easy to just get a feature to work, but it needs to be also maintained in the future, robust therefore well tested and validated, intuitive for other developers to use, well documented, and not cause a forseeable breaking change in the near future.
126+
127+
### Review Validity
128+
129+
> *The reviewer has never worked on the issue and was never part of any previous discussion, why would I care about their opinion?*
130+
131+
It's contrary to an open, collaborative environment to expect others to be involved in an issue or discussion since its beginning. Such a mindset would close out any new views, which are important for a differentiated discussion.
132+
133+
> *The reviewer doesn't have any expertise in that matter, why would I care about their opinion?*
134+
135+
Your arguments must focus on the issue, not on your assumption of someone else's personal experience. We will take immediate and appropriate action in case of personal attacks, regardless of your previous contributions. Personal attacks are not permissible. If you became a victim of personal attacks, you can privately [report](https://docs.github.com/en/communities/maintaining-your-safety-on-github/reporting-abuse-or-spam) the GitHub comment to the Parse Platform PMC.
136+
66137
## Environment Setup
67138

68139
### Recommended Tools
@@ -140,7 +211,7 @@ If your pull request introduces a change that may affect the storage or retrieva
140211
- `it_only_db('mongo')` // will make a test that only runs on mongo
141212
- `it_exclude_dbs(['postgres'])` // will make a test that runs against all DB's but postgres
142213
* Similarly, if your feature is intended to only work with PostgreSQL, you should disable MongoDB-specific tests with:
143-
214+
144215
- `describe_only_db('postgres')` // will create a `describe` that runs only on postgres
145216
- `it_only_db('postgres')` // will make a test that only runs on postgres
146217
- `it_exclude_dbs(['mongo'])` // will make a test that runs against all DB's but mongo
@@ -228,15 +299,15 @@ Adding a new security check for your feature is easy and fast:
228299
title: 'Door locked',
229300
warning: 'Anyone can enter your house.',
230301
solution: 'Lock the door.',
231-
check: () => {
302+
check: () => {
232303
return; // Example of a passing check
233304
}
234305
}),
235306
new Check({
236307
title: 'Camera online',
237308
warning: 'Security camera is offline.',
238309
solution: 'Check the camera.',
239-
check: async () => {
310+
check: async () => {
240311
throw 1; // Example of a failing check
241312
}
242313
}),
@@ -289,7 +360,7 @@ Introducing new [Parse Server configuration][config] parameters requires the fol
289360
2. If the new parameter does not have one single value but is a parameter group (an object containing multiple sub-parameters):
290361
- add the environment variable prefix for the parameter group to `nestedOptionEnvPrefix` in [/resources/buildConfigDefinition.js](https://github.com/parse-community/parse-server/blob/master/resources/buildConfigDefinition.js)
291362
- add the parameter group type to `nestedOptionTypes` in [/resources/buildConfigDefinition.js](https://github.com/parse-community/parse-server/blob/master/resources/buildConfigDefinition.js)
292-
363+
293364
For example, take a look at the existing Parse Server `security` parameter. It is a parameter group, because it has multiple sub-parameter such as `checkGroups`. Its interface is defined in [index.js][config-index] as `export interface SecurityOptions`. Therefore, the value to add to `nestedOptionTypes` would be `SecurityOptions`, the value to add to `nestedOptionEnvPrefix` would be `PARSE_SERVER_SECURITY_`.
294365

295366
3. Execute `npm run definitions` to automatically create the definitions in [/src/Options/Definitions.js][config-def] and [/src/Options/docs.js][config-docs].
@@ -299,6 +370,8 @@ Introducing new [Parse Server configuration][config] parameters requires the fol
299370

300371
## Pull Request
301372

373+
### Commit Message
374+
302375
For release automation, the title of pull requests needs to be written in a defined syntax. We loosely follow the [Conventional Commits](https://www.conventionalcommits.org) specification, which defines this syntax:
303376

304377
```
@@ -348,7 +421,7 @@ If the pull request contains a breaking change, the commit message must contain
348421
349422
```
350423
fix: remove handle from door
351-
424+
352425
BREAKING CHANGE: You cannot open the door anymore by using a handle. See the [#migration guide](http://example.com) for more details.
353426
```
354427
Keep in mind that in a repository with release automation, merging such a commit message will trigger a release with a major version increment.
@@ -359,7 +432,7 @@ If the commit reverts a previous commit, use the prefix `revert:`, followed by t
359432
360433
```
361434
revert: fix: remove handle from door
362-
435+
363436
This reverts commit 1234567890abcdef.
364437
```
365438
@@ -375,22 +448,92 @@ If the commit reverts a previous commit, use the prefix `revert:`, followed by t
375448
instead of:
376449
```
377450
revert: ci: add something
378-
451+
379452
This reverts commit 1234567890abcdef.
380453
```
381454
455+
### Security Vulnerability
456+
457+
#### Local Testing
458+
459+
Fixes for securify vulnerabilities are developed in private forks with a closed audience, inaccessible to the public. A current GitHub limitation does not allow to run CI tests on pull requests in private forks. Whether a pull requests fully passes all CI tests can only be determined by publishing the fix as a public pull request and running the CI. This means the fix and implicitly information about the vulnerabilty are made accessible to the public. This increases the risk that a vulnerability fix is published, but then cannot be merged immediately due to a CI issue. To mitigate that risk, before publishing a vulnerability fix, the following tests needs to be run locally and pass:
460+
461+
- `npm run test` (MongoDB)
462+
- `npm run test` (Postgres)
463+
- `npm run madge:circular` (circular dependencies)
464+
- `npm run lint` (Lint)
465+
- `npm run definitions` (Parse Server options definitions)
466+
467+
#### Merging
468+
469+
A current GitHub limitation does not allow to customize the commit message when merging pull requests of a private fork that was created to fix a security vulnerabilty. Our release automation framework demands a specific commit message syntax which therefore cannot be met. This prohibits to follow the process that GitHub suggest, which is to merge a pull request from a private fork directly to a public branch. Instead, after [local testing](#local-testing), a public pull request needs to be created with the code fix copied over from the private pull request.
470+
471+
This creates a risk that a vulnerability is indirectly disclosed by publishing a pull request with the fix, but the fix cannot be merged due to a CI issue. To mitigate that risk, the pull request title and description should be kept marginal or generic, not hiting to a vulnerabilty or giving any details about the vulnerabilty, until the pull request has been successfully merged.
472+
473+
## Releasing
474+
475+
### General Considerations
476+
477+
- The `package-lock.json` file has to be deleted and recreated by npm from scratch in regular intervals using the `npm i` command. It is not enough to only update the file via automated security pull requests (e.g. dependabot, snyk), that can create inconsistencies between sub-dependencies of a dependency and increase the chances of vulnerabilities. The file should be recreated once every release cycle which is usually monthly.
478+
382479
### Major Release / Long-Term-Support
383480
384-
Long-Term-Support (LTS) is provided for the previous Parse Server major version. For example, Parse Server 4.x will receive security updates until Parse Server 5.x is superseded by Parse Server 6.x and becomes the new LTS version. While the current major version is published on branch `release`, a LTS version is published on branch `release-#.x.x`, for example `release-4.x.x` for the Parse Server 4.x LTS branch.
481+
While the current major version is published on branch `release`, a Long-Term-Support (LTS) version is published on branch `release-#.x.x`, for example `release-4.x.x` for the Parse Server 4.x LTS branch.
385482
386-
#### Preparing Release
483+
### Preparing Release
387484
388485
The following changes are done in the `alpha` branch, before publishing the last `beta` version that will eventually become the major release. This way the changes trickle naturally through all branches and code consistency is ensured among branches.
389486
390487
- Make sure all [deprecations](https://github.com/parse-community/parse-server/blob/alpha/DEPRECATIONS.md) are reflected in code, old code is removed and the deprecations table is updated.
391488
- Add the future LTS branch `release-#.x.x` to the branch list in [release.config.js](https://github.com/parse-community/parse-server/blob/alpha/release.config.js) so that the branch will later be recognized for release automation.
392489
393-
#### Publishing Release
490+
491+
### Publishing Release (forward-merge):
492+
493+
1. Create new temporary branch `build` on branch `beta`.
494+
2. Create PR to merge `build` into `release`:
495+
- PR title: `build: release`
496+
- PR description: (leave empty)
497+
3. Resolve any conflicts:
498+
- For conflicts regarding the package version in `package.json` and `package-lock.json` it doesn't matter which version is chosen, as the version will be set by auto-release in a commit after merging. However, for both files the same version should be chosen when resolving the conflict.
499+
4. Merge PR with a "merge commit", do not "squash and merge":
500+
- Commit message: (use PR title)
501+
- Description: (leave empty)
502+
5. Wait for GitHub Action `release-automated` to finish:
503+
- If GitHub Action fails, investigate why; manual correction may be needed.
504+
6. Pull all remote branches into local branches.
505+
7. Delete temporary branch `build`.
506+
8. Create new temporary branch `build` on branch `alpha`.
507+
9. Create PR to merge `build` into `beta`:
508+
- PR title: `build: release`
509+
- PR description: (leave empty)
510+
8. Repeat steps 3-7 for PR from step 9.
511+
512+
### Publishing Hotfix (back-merge):
513+
514+
1. Create PR to merge hotfix PR into `release`:
515+
- Merge PR following the same rules as any PR would be merged into the working branch `alpha`.
516+
2. Wait for GitHub Action `release-automated` to finish:
517+
- GitHub Action will fail with error `! [rejected] HEAD -> beta (non-fast-forward)`; this is expected as auto-release currently cannot fully handle back-merging; docker will not publish the new release, so this has to be done manually using the GitHub workflow `release-manual-docker` and entering the version tag that has been created by auto-release.
518+
3. Pull all remote branches into local branches.
519+
4. Create a new temporary branch `backmerge` on branch `release`.
520+
5. Create PR to merge `backmerge` into `beta`:
521+
- PR title: `refactor: <commit-summary>` where `<commit-summary>` is the commit summary of step 1. The commit type needs to be `refactor`, otherwise the commit will show in the changelog of the `release` branch, once the `beta` branch is merged into release; this would a duplicate entry because the same changelog entry has already been generated when the PR was merged into the `release` branch in step 1.
522+
- PR description: (leave empty)
523+
6. Resolve any conflicts:
524+
- During back-merging, usually all changes are preserved; current changes come from the hotfix in the `release` branch, the incoming changes come from the `beta` branch usually being ahead of the `release` branch. This makes back-merging so complex and bug-prone and is the main reason why it should be avoided if possible.
525+
7. Merge PR with "squash and merge", do not do a "merge commit":
526+
- Commit message: (use PR title)
527+
- Description: (leave empty)
528+
529+
ℹ️ Merging this PR will not trigger a release; the back-merge will not appear in changelogs of the `beta`, `alpha` branches; the back-merged fix will be an undocumented change of these branches' next releases; if necessary, the change needs to be added manually to the pre-release changelogs *after* the next pre-releases.
530+
8. Delete temporary branch `backmerge`.
531+
10. Create a new temporary branch `backmerge` on branch `beta`.
532+
11. Repeat steps 4-8 to merge PR into `alpha`.
533+
534+
⚠️ Long-term-support branches are excluded from the processes above and handled individually as they do not have pre-releases branches and are not considered part of the current codebase anymore. It may be necessary to significantly adapt a PR for a LTS branch due to the differences in codebase and CI tests. This adaption should be done in advance before merging any related PR, especially for security fixes, as to not publish a vulnerability while it may still take significant time to adapt the fix for the older codebase of a LTS branch.
535+
536+
### Publishing Major Release (Yearly Release)
394537
395538
1. Create LTS branch `release-#.x.x` off the latest version tag on `release` branch.
396539
2. Create temporary branch `build-release` off branch `beta` and create a pull request with `release` as the base branch.

0 commit comments

Comments
 (0)