Problem/Motivation

core/lib/Drupal/Core/File/FileSystem.php contains lots of comments that are not necessary.

Proposed resolution

Remove unnecessary comments. See inline commenting standards https://www.drupal.org/docs/develop/standards/api-documentation-and-comm...

Remaining tasks

Do it.

User interface changes

None.

Original summary

Follow up to #2244513: Move the unmanaged file APIs to the file_system service (file.inc) There were a number of comments that were copy/pasted from the original.

+++ b/core/lib/Drupal/Core/File/FileSystem.php
@@ -301,4 +307,331 @@ public function validScheme($scheme) {
+    // Assert that the source file actually exists.
+    if (!file_exists($source)) {
...
+    // Check if directory exists.
+    if (!is_dir($directory)) {

I think we should remove comments like this - they add no value.

Let's add a follow-up to check all the comments in this file because I these are copy pastes.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kim.pepper created an issue. See original summary.

kim.pepper’s picture

jibran’s picture

@kim.pepper this can be a novice issue. Please update the IS for novice users and then we can tag it.

kim.pepper’s picture

Issue summary: View changes
Issue tags: +Novice

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

vadim.hirbu’s picture

Status: Active » Needs review
Akanksha92’s picture

@vadim.hirbu the patch did not apply properly. Please find the attached screenshot for the same.

Akanksha92’s picture

Assigned: Unassigned » Akanksha92
Status: Needs review » Needs work

Working on it as there are more comments that are not necessary

amit.drupal’s picture

Status: Needs work » Needs review

@Akanksha92

Patch is working fine and apply clearly.


  amit@VI-CPU-035:/var/www/html/drupal-stage$ patch -p1 < cleanup-comments-3033942-5.patch 
  patching file core/lib/Drupal/Core/File/FileSystem.php
  Hunk #1 succeeded at 314 with fuzz 1 (offset 1 line).
  Hunk #3 succeeded at 542 (offset -5 lines).
  Hunk #4 succeeded at 550 (offset -5 lines).
Darren Oh’s picture

Issue tags: +Seattle2019
RichardDavies’s picture

Rerolled against Drupal 8.8.x

kim.pepper’s picture

Status: Needs review » Reviewed & tested by the community

LGTM

larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Committed e3aa75b and pushed to 8.8.x. Thanks!

  • larowlan committed e3aa75b on 8.8.x
    Issue #3033942 by vadim.hirbu, RichardDavies: Clean up comments in...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.