- Issue created by @arti_parmar
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 1:30pm 21 June 2023 - last update
over 1 year ago 11 pass - Status changed to Needs work
over 1 year ago 9:08am 22 June 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
-getimagesize | X | | Caches calls to the PHP ```getimagesize()``` function. | +getimagesize | X | | Caches calls to the PHP ```getimagesize()``` +function.|
Since that is Markdown markup for a table, that line cannot be split in two.
- a. Use the _FileMetadataManagerInterface::class_ service to prepare collecting metadata for - the file located at a desired URI: + a. Use the _FileMetadataManagerInterface::class_ service to prepare + collecting metadata for the file located at a desired URI:
Since that line is part of a list, the second line must be indented.
- foreach ($my_file_metadata->getSupportedKeys('exif', ['ifds' => TRUE]) as $ifd) { + foreach ($my_file_metadata->getSupportedKeys('exif', + ['ifds' => TRUE]) as $ifd) {
Control structure conditions must go in a single line, as per current Drupal coding standards.
- $rows[] = ['data' => [$key[1], $x ? $x['text'] : NULL, $x ? var_export($x['value'], TRUE) : NULL]]; + $rows[] = ['data' => [$key[1], $x ? $x['text'] : NULL, + $x ? var_export($x['value'], TRUE) : NULL]];
Code lines are not required to be shorter than 81 characters. Line length and wrapping → , part of the Drupal coding standards, says:
- Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 characters.
- Control structure conditions may exceed 80 characters, if they are simple to read and understand.
(Emphasis is mine.)
/** - * @param string|null $uri - * The URI of the file. - * @param string $hash - * The hash used to reference the URI by file_mdm. + * {@inheritdoc} */ public function __construct(
{@inheritdoc}
is not used in documentation comments for constructors.
This makes me wonder: Was{@inheritdoc}
correctly used for the methods that can use it?+ /** + * Constructor. + * + * @param Traversable $namespaces + * An object that implements \Traversable which contains the root paths + * keyed by the corresponding namespace to look for plugin implementations. + * @param \Drupal\Core\Cache\CacheBackendInterface $cache + * The cache backend to be used. + * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler + * The module handler. + * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory + * The factory for configuration objects. + */
The description for a constructor must start with
Constructs a new
followed by the class name (including its namespace), and end withobject.
+ /** + * Module extension list. + *
A definite article is missing in the description.
It is not a list, but a service. - Status changed to RTBC
over 1 year ago 9:29am 22 June 2023 - Status changed to Needs work
over 1 year ago 1:01pm 22 June 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
The user who provided the patch/MR cannot change the status to Reviewed & tested by the community. Furthermore, since the status is Needs work, a new patch/MR needs to be provided.
- Status changed to Needs review
over 1 year ago 11:41am 23 June 2023 - last update
over 1 year ago 11 pass - Status changed to Needs work
over 1 year ago 3:22pm 23 June 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
-Plugin | Read | Write | Description | +Plugin | Read | Write | Description| --------------|:----:|:-----:|--------------------------------------------------------------| exif | X | X | Uses the [PHP Exif Library](https://github.com/lsolesen/pel) to read/write EXIF information to image files, bypassing the limitations of the standard PHP Exif extensions which only provides read capabilities. Enable the _file_mdm_exif_ submodule to enable this plugin. | font | X | | Uses the [PHP Font Lib](https://github.com/PhenX/php-font-lib) to read font information from TTF/OTF/WOFF font files. Enable the _file_mdm_exif_ submodule to enable this plugin. | -getimagesize | X | | Caches calls to the PHP ```getimagesize()``` function. | +getimagesize | X | | Caches calls to the PHP ```getimagesize()``` function.|
0 | Width of the image | 1 | Height of the image | 2 | The _IMAGETYPE_*_ constant indicating the type of the image | -3 | Text string with the correct _height="yyy" width="xxx"_ string that can be used directly in an IMG tag | +3 | Text string with the correct _height="yyy" width="xxx"_ string +that can be used directly in an IMG tag | mime | The MIME type of the image | channels | 3 for RGB pictures and 4 for CMYK pictures | bits | The number of bits for each color |
Since that is Markdown markup for a table, it should not be changed.
- a. Use the _FileMetadataManagerInterface::class_ service to prepare collecting metadata for - the file located at a desired URI: + a. Use the _FileMetadataManagerInterface::class_ service to prepare + collecting metadata for the file located at a desired URI:
The existing text is already correct.
-2. __Use a known local temp copy of the remote file to avoid remote file access:__ +2. __Use a known local temp copy of the remote file to avoid remote +file access:__
Since that text is part of a list, the second line should be indented. Then, since that line should be bold, I am not sure it can be split on two lines.
+ $this->assertNull($file_metadata->getMetadata('exif', + [1, 'imagedescription']));
If the code is going to be split on two lines, the second line must be indented too, to make more evident that array is an argument of
$file_metadata->getMetadata()
.
Also, the parentheses should be placed on a new line, which is what has been done with the next changed lines.+ $desc_tag = $this->container->get(ExifTagMapperInterface::class)->resolveKeyToIfdAndTag([1, + 'imagedescription', + ]);
That is the correct way to format the code.
+ * Constructs a new FileMetadata Object. + * + * @param \Drupal\file_mdm\Plugin\FileMetadataPluginManagerInterface $pluginManager + * The plugin manager. + * @param \Psr\Log\LoggerInterface $logger + * The logger. + * @param \Drupal\Core\File\FileSystemInterface $fileSystem + * The file System. + * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory + * The config Factory.
The method description is missing the class namespace.
+ /** + * {@inheritdoc} + */ public function __construct(string $message, string $plugin_id = NULL, string $method = NULL, \Exception $previous = NULL) {
{@inheritdoc}
is not used in documentation comments for constructors.
The description for a constructor must start withConstructs a new
followed by the class name (including its namespace), and end withobject.
+ /** + * Module extension extension. + * + * @var \Drupal\Core\Extension\ModuleExtensionList + */ protected readonly ModuleExtensionList $moduleList;
The property description is not correct.
- Assigned to nitin_lama
- Status changed to Needs review
over 1 year ago 12:23pm 27 June 2023 - last update
over 1 year ago 11 pass - Issue was unassigned.
- First commit to issue fork.
- 🇮🇳India sidharth_soman Bangalore
Patch #9 applies cleanly and solves most of the issues. The only ones remaining are:
C:\xampp\htdocs\backendassignment\web\modules\contrib>phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig file_mdm-3368294 FILE: C:\xampp\htdocs\backendassignment\web\modules\contrib\file_mdm-3368294\file_mdm_exif\README.md ---------------------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES ---------------------------------------------------------------------------------------------------- 82 | WARNING | Line exceeds 80 characters; contains 83 characters 88 | WARNING | Line exceeds 80 characters; contains 107 characters ---------------------------------------------------------------------------------------------------- FILE: C:\xampp\htdocs\backendassignment\web\modules\contrib\file_mdm-3368294\README.md -------------------------------------------------------------------------------------- FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 5 LINES -------------------------------------------------------------------------------------- 12 | WARNING | Line exceeds 80 characters; contains 93 characters 16 | WARNING | Line exceeds 80 characters; contains 94 characters 77 | WARNING | Line exceeds 80 characters; contains 115 characters 84 | WARNING | Line exceeds 80 characters; contains 93 characters 114 | WARNING | Line exceeds 80 characters; contains 82 characters -------------------------------------------------------------------------------------- FILE: C:\xampp\htdocs\backendassignment\web\modules\contrib\file_mdm-3368294\src\Plugin\FileMetadataPluginManager.php --------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE --------------------------------------------------------------------------------------------------------------------- 32 | ERROR | Expected type hint "Traversable"; found "\Traversable" for $namespaces --------------------------------------------------------------------------------------------------------------------- Time: 6.91 secs; Memory: 8MB
I assume the above are to be left as it is for readability/accessibility purposes.
- Status changed to Closed: outdated
10 months ago 10:52pm 5 January 2024