Skip to content

Commit e7aebfb

Browse files
[TASK] Add note about type narrowing (#391)
* [TASK] Add note about type narrowing * [TASK] Add newline to properly parse headings
1 parent e1d693d commit e7aebfb

File tree

1 file changed

+13
-2
lines changed

1 file changed

+13
-2
lines changed

Documentation/HandlingAPatch/Tips.rst

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,31 +283,38 @@ For reviewing and giving feedback, here's a couple of things that are often addr
283283
use this list both for checking other people's patches, as well as your own.
284284

285285
.. _common-review-checks-general:
286+
286287
General checks
287288
~~~~~~~~~~~~~~
288289

289290
* Is the **code-flow readable**? Does it need more (or less) comments? Any **code complexities**
290291
("cognitive complexity") that could be easier to read when using different conditions/loops/sub-methods?
291292
* Do breaking changes occur that need to be noticed? This can also apply to:
292293

293-
* **Type hinting / type declarations**
294+
* **Type hinting / type declarations** - specifically, narrowing type
295+
declarations should usually not be backported to patchlevel
296+
versions (especially if it affects public methods/properties). Similarly,
297+
adding type-casting to variables declared as "pass by reference" in their
298+
method signatures should be prevented in backports.
294299
* using PHP features beyond the supported PHP version
295300
* Loss of existing functionality
296-
* Typos
301+
* Typos (specifically in variable/method/function/property names)
297302

298303
* Are new class, method, function, **variable names understandable**
299304
* Are possibilities for **early code returns** and reduced **nesting levels** addressed?
300305
* If **SCSS/TypeScript** changes are involved, are the resulting build files included in
301306
the patch, and were built with the right environment?
302307

303308
.. _common-review-checks-testing:
309+
304310
Testing
305311
~~~~~~~
306312

307313
* Is there a need to add **unit/functional testing** for specific changes
308314
* Are **regression tests** for a bugfix needed?
309315

310316
.. _common-review-checks-formalities:
317+
311318
Formalities
312319
~~~~~~~~~~~
313320

@@ -319,6 +326,7 @@ Formalities
319326
* When **new PHP files** are added, do they contain the TYPO3 License and a `declare strict_types` header?
320327

321328
.. _common-review-checks-php:
329+
322330
PHP specifics
323331
~~~~~~~~~~~~~
324332

@@ -327,6 +335,7 @@ PHP specifics
327335
* Is **Dependency Injection** used where applicable?
328336

329337
.. _common-review-checks-docs:
338+
330339
Documentation
331340
~~~~~~~~~~~~~
332341

@@ -340,6 +349,7 @@ Documentation
340349
* Does your patch **deprecate** anything? If so, have you followed :ref:`deprecations`?
341350

342351
.. _common-review-checks-xlf:
352+
343353
Xliff / language files
344354
~~~~~~~~~~~~~~~~~~~~~~
345355

@@ -374,6 +384,7 @@ When changes are made to **Xliff files** (translations):
374384
of spelling/grammar, but **never change in meaning or arguments**.
375385

376386
.. _common-review-checks-missing:
387+
377388
More?
378389
~~~~~
379390

0 commit comments

Comments
 (0)