We're always looking for community contributions to Concrete CMS. If a concrete issue is marked as available then it's ready to get a code submission.

We do try to maintain high standards for code that makes it into the concrete core – but that's because we want the core to stay stable and full-featured for editors and visitors, while remaining flexible for developers.

So, you've found an issue you'd like to write code for. Or there's no existing issue at all, and you'd like to submit code that we haven't specifically asked for. That's ok too! But you'll have to convince us that it belongs in the core. Here's how you can do that.

Main Points

• Keep complexity low. If people can't understand your feature / system people will not use it
• Keep confidence high. Clean tested code is code that you can be confident works
• Automate as much as possible. Automate your dev processes and review processes
• Code gets reviewed. Everyone is held to the same standards

Defects

All code has bugs. That's why we use reviews to catch them. Submitting pull requests that contain accidental bugs will not necessarily prevent you from having pull requests approved in the future, however if you intentionally submit code with defects for nefarious purposes, we reserve the right to bar you from the project.

Complexity

It's easy to over-think and over-architect things when working in complex systems. Ask yourself often if the code you're writing can be simpler or if features you're adding are really going to be used. Keep in mind that if other developers can't understand the things you're adding, they won't use them.

In that same vein, make sure the features you're adding are actually needed. If your pull requests specifically answers an issue that we've marked as available, there's a good chance that we're actively looking for a fix or an enhancement, and a much better chance that we'll accept your code.

Automation

Coding is hard, and keeping up with the standards is extra hard. Keep an eye out for ways you can automate things so that you don't have to actually worry about them.

• Code style! Don't do this manually, it's hard and painstaking if you do it right and it's really easy to do wrong. Configure your IDE to format as you write or on save, or use a command line utility to format your code before committing.
• Tests! Use your IDE to run your tests as you go and display your coverage

And don't stop at development, automate things during review so that small things are caught without a reviewer.

Some other good tools to use:

• Javascript
  • eslint
  • normaljs
• PHP
  • PHPCSFixer
  • Prettifier
  • PHPStan
  • Phan
  • PHPUnit
• Service
  • Github Actions
  • Travis CI
  • Scrutinizer CI
  • Circle CI

Wherever possible, use laravel mix to build assets and rely on the recommended scripts like npm run watch and npm run hot as much as possible.

Design

Code Design

Make sure your code is designed well:

• Does it use the existing features as intended? Are you following the available documentation for the features you use?
• Does it fit with the rest of the code in the project? Are you working around existing systems or working with them?
• Is it duplicating functionality? Make sure you're not rewriting things that are available elsewhere in the codebase.

Front end design

If the feature you're building comes with designs from a designer make sure your changes match. Use hot reloading and a tool like perfect pixel to get things dialed in. Reviewers will check against the comps so if you're unable to meet things exactly, describe why in your pull request.

Testing

There are several types of tests, we always want as much coverage as is possible!

• Unit tests: These test a small portion of code like a single method or a function
• Integrations tests: These test the integration between two different tools like one class and another or a database
• End to end tests: Tests that test the resulting output in the browser or device

Code Style

Keeping code style clean and consistent is an important part of reviewing code. Following a standard ensures that everyone will be able to easily read everyone elses code, and we keep the changelog clear of unnecessary formatting changes. A reviewer shouldn't need to dig through piles of whitespace changes or newline additions in order to review code.

PHP

PHP is the main language we use, it can be very forgiving when it comes to syntax so it's important to follow a standard so that things stay readable.

In general we follow the following standards:

• PSR-1: General stuff like class naming and tags
• PSR-12: Detailed things like how to format classes and operators
• PHPDoc: Docblock comments in code

Class files

Class files should only ever declare a single class, they shouldn't have any functionality beyond that.

<?php

namespace Concrete\Core\Something;

use Vendor\Package\SomeNamespace\ClassA as A;

/**
 * PHP Doc block describing this class
 */
class Foo extends Bar implements FooInterface
{

    /**
     * A short description of this sample function
     * Some additional details as needed
     *
     * @param int $firstArgument A description for this argument
     * @param int $secondArgument A description for the second argument
     *
     * @return array A description for the return value
     *
     * @throws SomeException A description for why/when this is thrown
     */
    public function sampleFunction(int $firstArgument, int $secondArgument = null): array
    {
        if ($firstArgument === $secondArgument) {
            bar();
        } elseif ($firstArgument > $secondArgument) {
            $foo->bar($secondArgument);
        } else {
            BazClass::bar($firstArgument, $secondArgument);
        }

        return [];
    }

    /**
     * A short description for bar
     */
    final public static function bar()
    {
        // method body
    }
}

Mixed PHP / HTML

Mixing PHP and HTML can lead to some really challenging formatting issues.

• Use the fewest amount of PHP opening and closing tags as possible, avoid ?><?php at all costs, especially if it has newlines in the middle.
• Put PHP opening and closing tags on their own lines unless you're echoing or doing a single statement
• Scope indent all PHP opening and closing tags to match scope
• If you're forced to choose one or the other, always use PHP's scope for indentation over HTMLs
• Things that have their own scope like conditionals or

<?php defined('C5_EXECUTE') or die('Access Denied') ?>

<div class="example">
    <?php
    if (!empty($value)) {
        ?>
        <span><strong>Value:</strong> <?= $value ?></span>
        <?php
    } else {
        ?>
        <span class="warning">Value is not set!</span>
        <?php
    }
    ?>
</div>

<?php
defined('C5_EXECUTE') or die('Access Denied');

$count = 0;
foreach ($values as $value) {
    if ($count++ % 3) {
        if ($count > 2) {
            ?>
            </ul>
            <?php
        }
        ?>
        <div class='row'>
            <ul>
        <?php
    }
    ?>
    <li class="value"><?= $value ?></li>
    <?php
}
?>

JavaScript (and the like)

We've always heavily relied on JS and we've seen and used a thousand different ways to format it. If PHP is easy to get wrong, JS takes that to the extreme.

Whether you're using typescript or javascript, in general follow standard JS with the following changes:

• space-before-function-paren disabled: Instead of function () {} we use function() {} to be more consistent with PHP
• indent 4: Use 4 spaces instead of 2 to be consistent with existing code.

Javascript

async function foo(foo) {
    const compare = await getAsyncronousThing()

    if (foo > compare) {
        return foo
    }

    return 0
}

Typescript

async function foo(foo: number): Promise<number> {
    const compare: number = await getAsyncronousThing()

    if (foo > compare) {
         return foo
    }

    return 0
}

class Bar {
    test: Array<string> = ['array', 'with', 'strings', 'in', 'it']

    foo(foo: number): number {
        return foo
    }
}

Vue

Use typescript with vue wherever possible and prefer class declarations over vue.extend syntax. Use annotations from vue-property-decorator and strong typing as much as possible.

<template>
<div>
    Count is <span v-html="currentCount"></span>
    <button @click="add" :disabled="currentCount > end">Add</button>
</div>
</template>
<script>
import { Vue, Component, Prop } from 'vue-property-decorator'

@Component
export default class Counter extends Vue {
    /** The starting value */
    @prop(Number) readonly startValue: number

    /** The maximum value */
    @prop(Number) readonly maxValue: number

    /** "added" count data item */
    added: number = 0
    /** Starting value data item */
    start: number = this.startValue
    /** Ending value data item */
    end: number = this.maxValue

    /**
     * Add a number to the count
     */
    add(): void {
        this.added++
    }

    /**
     * Return the current count
     */
    get currentCount(): number {
        return this.start + this.added
    }
}
</script>

SCSS / CSS

We follow sass guidelines when writing SCSS and we apply those same principles wherever possible to CSS. Prefer writing SCSS over straight CSS wherever possible.

// A description for a rule (this style of comment is only supported in scss)
.some-class {
  color: rgb(200, 10, 0);
  opacity: 0.5;
}

When building SCSS for the core in the Bedrock Asset System, make sure to try and keep your SCSS nicely contained. Need to add styles for the new "Summary Templates" panel? Don't add them to cms/scss/panels/_shared.scss. Instead, make a new include in cms/scss/panels/pages/_summary-templates.scss and include that from within cms/scss/_panels.scsss.

Code Review

Use CI tools like travis, scrutinizer, github actions, and circle to validate things like code style and tests passing. If it can be automated, automate it!

What should a reviewer look for?

• Clear pull request: Does it describe of all of the changes? Is it clear what has changed and what to look at? Is functionality to test documented?
• Front end design: If the task has a comp attached to it, does your pull request mirror the comp exactly, or within 99%? Would the designer agree? Are inconsistencies explained in the pull request?
• Code design: Does it match the rest of the code? Is it using existing functionality properly and not rebuilding things?
• Functionality: Does the code do what the requester says it does? Did the requester describe all the changes in the pull request?
• Complexity: Is it understandable? Can it be made simpler? Would other developers be able to use it?
• Tests: Is it tested? If not is there a good reason why not?
• Code style: Does it match our code style? Are names properly formatted with the right casing?
• Comments: Does the code have clear comments describing why things are happening? Is it going to be easy to come back to in 6 months?

Thank You!

Thank you for taking the time to read this, and for submitting code to Concrete. We are stronger for having such an active community, and we look forward to your code submissions.