Wrong @param doc in class Composer

Thanks for the patch! Sorry for the delay in reviewing -- I've been on vacation and no one else, unfortunately, ever reviews docs patches. :(

+++ b/core/lib/Drupal/Core/Composer/Composer.php
@@ -142,7 +142,7 @@ public static function ensureHtaccess(Event $event) {
+   * @param \Composer\Installer\PackageEvent $event

This also needs a documentation line added.

Thanks!

+++ b/core/lib/Drupal/Core/Composer/Composer.php
@@ -142,7 +142,8 @@ public static function ensureHtaccess(Event $event) {
+   *   A script event object from composer.

Hm. OK, not sure what this means, but presumably "composer" should be capitalized at least?

And if this is of class PackageEvent, why is it documented as 'a script event" object? It might be right, but please check. I would think it's a "package" event, whatever that is.

+++ b/core/lib/Drupal/Core/Composer/Composer.php
@@ -125,6 +125,8 @@ public static function ensureHtaccess(Event $event) {
+   *  A script PackageEvent object from composer.

composer is still not capitalized here. Where is the interdiff ?

Please work on the assigned issue this issue is already assigned to rakesh.

Assigning to myself due to inactivity.

Please review the attached patch. I just started from #4

Thanks for the new patch!

+++ b/core/lib/Drupal/Core/Composer/Composer.php
@@ -142,7 +142,8 @@ public static function ensureHtaccess(Event $event) {
    * Remove possibly problematic test files from vendored projects.
    *
-   * @param \Composer\Script\Event $event
+   * @param \Composer\Installer\PackageEvent $event
+   *   A PackageEvent object from Composer.
    */
   public static function vendorTestCodeCleanup(PackageEvent $event) {

Hm...

So this @param documentation is probably correct, but it doesn't tell me anything that I cannot read from the function signature. I have no idea what a PackageEvent object is, or why you would need to pass one into this function, or what it is used for.

So... although it is not incorrect, I don't think this is very good documentation. If we're going to fix up this @param, let's figure out what this really is, why it's needed, and what it's used for, and put that into the documentation. OK? Thanks!

So you want me to change @param documentation as well as function signature's work ?

The issue title is "Wrong @param doc". Yes, change the param doc. :) I do not think we are changing the function signature.

Hi All,
I have created a patch, though its not perfect, its a start and open for others to further enhance it.
About the PackageEvent, its a class that extends InstallerEvent class, which is An event for all installer. It passed any event related to installer as an argument. Kindly provide more suggestions for this.

+++ b/core/lib/Drupal/Core/Composer/Composer.php
@@ -142,7 +142,10 @@ public static function ensureHtaccess(Event $event) {
+   * @param \Composer\Installer\PackageEvent $event
+   *   A PackageEvent object from Composer.
+   *   PackageEvent extends InstallerEvent class, which is an event for all
+   *   installer.

Formatting: The lines after @param need to be wrapped into a nice paragraph, as close to 80 character lines as possible.

Documentation... So when you are documenting a parameter, the aim is to document what the parameter is used for in the method, or how it affects the outcome of the method.

This documentation doesn't tell me that. It is just trying to tell me what the PackageEvent class is, which I could/should get from reading the class documentation, not from reading a @param documentation line.

So what we need to document here is what $event is used for in the function.

This function's one-line intro says:

Remove possibly problematic test files from vendored projects.

So look at the code and figure out how $event is related to that process, and put that into the documentation.

Thanks!

Hello,
I have tried to add some documentation for the event object, Please review my patch.

Sorry for the previous attachments Please ignore that. I am uploading corrected patch and interdiff files. I have taken the last patch and added my changes according to that.

Ah, OK, that is better -- it actually tells me what $event is being used for. Thank you!

Now, let's fix the standards/formatting:

+++ b/core/lib/Drupal/Core/Composer/Composer.php
@@ -142,7 +142,9 @@ public static function ensureHtaccess(Event $event) {
+   * A PackageEvent object to get the current working directory for composer
+   * scripts to run.

The documentation lines need to be indented by 2 spaces.

Also... maybe a bit of rewriting... I was able to figure out what you were trying to say, but it doesn't really make sense as it is written.

Maybe something like:

A PackageEvent object to get the configured composer vendor directories from.

??? And I didn't read the rest of the code in the module to see if $event is used for anything other than that first line.

Long time, i am back with new energy from DrupalCon Asia.
Here is the patch and interdiff.

Thanks! Almost there, but:

+++ b/core/lib/Drupal/Core/Composer/Composer.php
@@ -142,7 +142,9 @@ public static function ensureHtaccess(Event $event) {
+   *   A PackageEvent object to get the configured composer vendor
+   *   directories from.

Wrapping nitpick: The word "directories" will fit on the previous line.

Sorry for silly mistake.
Here is the final patch with interdiff.

Looks good now, thanks! We all (even me, frequently!) make wrapping mistakes from time to time -- this is why we have reviews. ;)

Thanks for patience and time.

Committed/pushed to 8.2.x and cherry-picked to 8.1.x and 8.0.x. Thanks!