Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-43233

POLICY: Accepting commits which only fix coding style



    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 2.7
    • None
    • Policy


      "Coding style change" refer to the situation when inline comments, spaces, indentation, variable/funciton names are changed to comply with Moodle coding styles.
      This issue is not about phpdocs changes.

      Can we accept such changes in the following cases:

      1. Several lines are changed to compy with Coding Style in the same commit as bug fix/improvement and are in the same piece of code as the actual fix.

      2. Many coding style lines are changed in the same file as actual bug fix/improvement.

      • in the same commit as bug fix
      • in the separate commit

      3. Coding style is improved in the file that was not affected in the actual bug fix/improvement

      4. There is a separate issue that changes the coding styles only

      • in one often used library file or accompanies the issue with bug fix in the same file
      • massive change in many files
      • the issue also contains unittests or behat tests that ensure that each changed file/function is tested
      • the issue changes the files that were not modified for a long time

      5. "Change that does not break anything if accidentally backported in some cherry-pick of bugfix".

      • for example: change function call $OUTPUT->BOX() to $OUTPUT->box()

      Cons of coding style changes:

      • possibility of creating a new bug
      • possibility of creating problem later when working on bug and changing the same lines (for example, new variable name is cherry-picked into the stable version)
      • git blame does not show actual code changes


        Issue Links



              stronk7 Eloy Lafuente (stronk7)
              marina Marina Glancy
              David Woloszyn, Huong Nguyen, Jake Dallimore, Meirza, Michael Hawkins, Raquel Ortega, Safat Shahin, Stevani Andolo
              2 Vote for this issue
              11 Start watching this issue