From 44a4abdfbd1bd0fc1c831ead42718ed66f27bbaf Mon Sep 17 00:00:00 2001 From: Laura Neto <12862535+lauraneto@users.noreply.github.com> Date: Tue, 3 Oct 2023 18:44:10 +0200 Subject: [PATCH] Updating contributing docs to add unwanted changes section (#14853) * Updating contributing docs to add unwanted changes section * Added new section to table of contents --- .github/CONTRIBUTING.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index c0eaf680a3..06fd873638 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -19,6 +19,7 @@ We also encourage community members to feel free to comment on others' pull requ * [What can I contribute?](#what-can-i-contribute) + [Making larger changes](#making-larger-changes) + [Pull request or package?](#pull-request-or-package) + + [Unwanted changes](#unwanted-changes) + [Ownership and copyright](#ownership-and-copyright) - [Finding your first issue: Up for grabs](#finding-your-first-issue-up-for-grabs) - [Making your changes](#making-your-changes) @@ -69,6 +70,25 @@ If you're unsure about whether your changes belong in the core Umbraco CMS or if If it doesn’t fit in CMS right now, we will likely encourage you to make it into a package instead. A package is a great way to check out popularity of a feature, learn how people use it, validate good usability and fix bugs. Eventually, a package could "graduate" to be included in the CMS. +#### Unwanted changes +While most changes are welcome, there are certain types of changes that are discouraged and might get your pull request refused. +Of course this will depend heavily on the specific change, but please take the following examples in mind. + +- **Breaking changes (code and/or behavioral) 💥** - sometimes it can be a bit hard to know if a change is breaking or not. Fortunately, if it relates to code, the build will fail and warn you. +- **Large refactors 🤯** - the larger the refactor, the larger the probability of introducing new bugs/issues. +- **Changes to obsolete code and/or property editors ✍️** +- **Adding new config options 🦾** - while having more flexibility is (most of the times) better, having too many options can also become overwhelming/confusing, especially if there are other (good/simple) ways to achieve it. +- **Whitespace changes 🫥** - while some of our files might not follow the formatting/whitespace rules (mostly old ones), changing several of them in one go would cause major merge conflicts with open pull requests or other work in progress. Do feel free to fix these when you are working on another issue/feature and end up "touching" those files! +- **Adding new extension/helper methods ✋** - keep in mind that more code also means more to maintain, so if a helper is only meaningful for a few, it might not be worth adding it to the core. + +While these are only a few examples, it is important to ask yourself these questions before making a pull request: + +- How many will benefit from this change? +- Are there other ways to achieve this? And if so, how do they compare? +- How maintainable is the change? +- What would be the effort to test it properly? +- Do the benefits outweigh the risks? + #### Ownership and copyright It is your responsibility to make sure that you're allowed to share the code you're providing us. For example, you should have permission from your employer or customer to share code.