>>> [!note] Migrated issue <!-- Drupal.org comment --> <!-- Migrated from issue #1296842. --> Reported by: [sun](https://www.drupal.org/user/54136) >>> <h3>Problem</h3> <ul> <li>Uncertainty about when or how to apply coding standards across Drupal core versions and core vs. contrib code.</li> </ul> <h3>Goal</h3> <ul> <li>Clarify the scope of coding standards with regard to <ol> <li>major versions of core</li> <li>core vs. contrib code</li> </ol> </li> <li>Explicitly specify/state which particular coding standards have been different for earlier versions.</li> </ul> <h3>Details</h3> <ul> <li>In <span class="drupalorg-gitlab-issue-link project-issue-status-info project-issue-status-3"><a href="https://www.drupal.org/project/drupal/issues/1290258" title="Status: Closed (duplicate)">#1290258: The data type should be explicitly stated for all Drupal core @param and @return documentation.</a></span> some (legit) confusion arose around my statement in a <strong>contrib</strong> <a href="http://drupal.org/node/1159356#comment-4560546">issue</a>:<br> <blockquote><p> Drupal coding standards are version-independent and "always-current". All new code should follow the current standards, regardless of (core) version. Existing code in older versions may be updated, but doesn't necessarily have to be. Especially for larger code-bases (like Drupal core), updating the code of a previous version for current standards may too huge of a task. However, code in current versions should follow the current standards. Lastly, when following this practice, make sure to not squeeze coding standards updates/clean-ups into otherwise unrelated patches. </p></blockquote> </li> <li>Based on that, the main people caring for Drupal coding standards discussed it and added the following intro text to <a href="http://drupal.org/coding-standards:">http://drupal.org/coding-standards:</a><br> <blockquote><p> Drupal coding standards are version-independent and "always-current". All new code should follow the current standards, regardless of (core) version. Existing code in older versions <em>may</em> be updated, but doesn't necessarily have to be. Especially for larger code-bases (like Drupal core), updating the code of a previous version for the current standards may too huge of a task. However, code in current versions should follow the current standards.</p> <p><em>Note: Do not squeeze coding standards updates/clean-ups into otherwise unrelated patches. Only touch code lines that are actually relevant. To update existing code for the current standards, always create separate and dedicated issues and patches.</em> </p></blockquote> <p>Based on the confusion that arose in aforementioned issue, this could have probably been discussed some more. ;) </p></li> <li>As determined in aforementioned issue, there are coding standards - e.g., for string concatenation - which we currently apply differently for <em>Drupal core patches</em> depending on the core version a patch is for. <p>I.e., patches backported from D7 to D6 <em>may</em> be adjusted for the differing string concatenation rules being used throughout Drupal 6 core for consistency.</p> <p><em>(Note, however, that I (sun) am currently not sure whether this "should" is actually turned into a "required" for backported D6 patches.)</em> </p></li> <li>The above coding standards intro text was primarily intended to clarify the scope of current coding standards in <em>contributed extensions</em>, and also clarify how to update (or ignore) existing code.</li> </ul> <h3 id="summary-proposed-resolution">Proposed resolutions <em>(with pros and cons)</em></h3> <ol> <li id="resolution-version-standards"> <dl> <dt>Require all code to meet version-specific standards.</dt> <dd> <p><strong>Pro:</strong></p> <ul> <li> <p>Helps maintain consistency. D6 patches should be written to D6 standards, D7 patches should be written to D7 standards, and D8 patches to (current) D8 standards.</p> </li> </ul> <p><strong>Con:</strong></p> <ul> <li> <p>We have no currently-published version-specific standards, and no easy way to produce them.</p> </li> </ul> </dd> </dl> </li> <li id="resolution-surrounding-code"> <dl> <dt>Require all code to be consistent with the surrounding code, regardless of standards.</dt> <dd> <p><strong>Pro:</strong></p> <ul> <li> <p>Helps maintain consistency. If there is any question as to coding standards, use the surrounding code as a standards guide.</p> </li> </ul> <p><strong>Con:</strong></p> <ul> <li> <p>The surrounding code, despite huge efforts at standardization, often lacks sufficient consistency to serve as a standard.</p> </li> </ul> </dd> </dl> </li> <li id="resolution-current-standards"> <dl> <dt>Require all code to meet currently-published standards, regardless of surrounding code.</dt> <dd> <p><strong>Pro:</strong></p> <ul> <li> <p>Makes it easier to backport patches. If the style is good enough for D8, then it's good enough for D7 and D6, as well.</p> </li> </ul> <p><strong>Con:</strong></p> <ul> <li> <p>If the backported patch was written to a newer, different standard, it may not be consistent in style with the surrounding code. This will introduce the very type of inconsistencies that standards are written to avoid.</p> </li> </ul> </dd> </dl> <dl> <dt id="resolution-current-standards-with-exceptions">3.1 &mdash; Same as above, with certain well-known exceptions.</dt> <dd> <p><em>(@todo)</em> The exceptions, no matter how well-known among senior core developers, should be documented for the sake of the rest of the community.</p> </dd> </dl> </li> <li id="resolution-coder-review"> <dl> <dt>Require all code to pass coder review as written for the target branch of core.</dt> <dd> <p><strong>Pro:</strong></p> <ul> <li> <p>This allows automation of the stylistic part of patch review, and also enforces version-specific standards which should help keep D8 patches consistent with D8, D7 patches consistent with D7, and D6 patches consistent with D6.</p> </li> </ul> <p><strong>Con:</strong></p> <ul> <li> <p>Coder review needs more active maintainers before this option can be considered.</p> </li> </ul> </dd> </dl> </li> <li id="resolution-common-sense"> <dl> <dt>Use a common-sense approach.</dt> <dd> <p><strong>Pro:</strong></p> <ul> <li> <p>Allows us to decide these issues on a case-by-case basis instead of trying to come up with a Single Golden Rule That Covers All Possible Cases.</p> </li> </ul> <p><strong>Con:</strong></p> <ul> <li> <p>Even people who have "common sense" often disagree on what it means. The reason for developing rules and standards is to hear all the arguments <strong>once</strong>, then record the resultant decision so further disagreement is reduced or eliminated.</p> </li> </ul> </dd> </dl> </li> </ol> <p>In <a href="http://drupal.org/node/1296842#comment-5074792">#23</a>, <a href="http://drupal.org/user/24967">webchick</a> postponed this issue until blockers to <a href="http://drupal.org/node/1296842#resolution-coder-review">#4: Use Coder Review</a> are resolved.</p> <h3 id="summary-remaining-tasks">Remaining Tasks</h3> <p>See: <span class="drupalorg-gitlab-issue-link project-issue-status-info project-issue-status-7"><a href="https://www.drupal.org/project/drupalci/issues/1299710" title="Status: Closed (fixed)">#1299710: [meta] Automate the coding-standards part of patch review</a></span></p>