- Issue created by @himanshu_jhaloya
- @himanshu_jhaloya opened merge request.
- Issue was unassigned.
- Status changed to Needs review
almost 2 years ago 10:07am 2 May 2023 - Status changed to Needs work
almost 2 years ago 11:27am 2 May 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
/** - * Class AparatApi + * Class of AparatApi. */
A class short description must not start with Class of nor repeat the class name.
/** + * Logger channel interface. + * * @var \Drupal\Core\Logger\LoggerChannel */ protected LoggerChannel $logger;
There is a missing article. interface is not necessary and it is wrong, since a property cannot contain an interface.
Each description is missing an article. Since that documentation comment is edited, the constructor description needs to be changed too. <code> /** - * get Video info from video hash + * Get Video info from video hash. * * @param string $hash - * The vide hash + * The vide hash. * * @return array|bool + * Return array. */ public function getVideoInfo(string $hash) {
In the short description, the verb must use the third person singular.
vide is a typo.
The return value description must describe the return value, not its type.try { - $resource_url = $this->urlResolver->getResourceUrl($url); - $this->resourceFetcher->fetchResource($resource_url); + $resource_url = $this->urlResolver->getResourceUrl($url); + $this->resourceFetcher->fetchResource($resource_url); }
The code is already correctly indented, contrary to the changed code.
/** + * Third party api. + * * @var \Drupal\media_aparat\AparatApi */
api is misspelled.
That description is too vague.- * @param \Drupal\media_aparat\AparatApi $aparat_api + * @param \Drupal\media_aparat\AparatApi $aparatApi + * The third party api. * @param \Drupal\Core\Logger\LoggerChannelFactoryInterface $logger_factory + * The logger service. */
Local variables, which include also method parameters, are allowed to follow that naming schema.
/** - * Prevent usage of this field formatter on any fields other than those which are added in - * a media type which it's source is AparatVideo + * Prevent usage of this field formatter on any fields other than. + * + * Those which are added in a media type which it's source is AparatVideo.
Prevent usage of this field formatter on any fields other than. alone is not a sentence.
Those which are added in a media type which it's source is AparatVideo. does not make any sense and it is not grammatically correct.
The existing description must be fixed, but the changed description is not the way to correct it./** - * Prevent usage of this field widget on any fields other than those which are added in - * a media type which it's source is AparatVideo + * Prevent usage of this field widget on any fields other than . + * + * Those which are added in a media type which it's source is AparatVideo. */
Ditto.
/** - * The Aparat Api + * The Aparat Api. + * + * @var \Drupal\media_aparat\AparatApi */ protected AparatApi $aparatApi;
Api is misspelled.
- ๐ฎ๐ณIndia ashutosh ahirwal India
Ashutosh Ahirwal โ made their first commit to this issueโs fork.
- Status changed to Needs review
almost 2 years ago 12:13pm 22 June 2023 - ๐ฎ๐ณIndia ashutosh ahirwal India
Hi I have addressed #5 comment and updated the MR
Please review. - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
(Fixing code that does not follow the Drupal coding standards is a task.)
- Status changed to Needs work
almost 2 years ago 12:28pm 22 June 2023 - Assigned to nitin_lama
- First commit to issue fork.
- Issue was unassigned.
- Status changed to Needs review
about 1 year ago 8:31am 15 February 2024 - Status changed to Needs work
9 months ago 11:34pm 15 July 2024 Hi @everyone,
Applied latest changes on MR, it applied not-so successfully and still threw multiple errors. Please see below:
media_aparat git:(2.0.x) curl https://git.drupalcode.org/project/media_aparat/-/merge_requests/1.diff | patch -p1 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 12047 0 12047 0 0 24663 0 --:--:-- --:--:-- --:--:-- 25097 patching file media_aparat.info.yml Hunk #1 FAILED at 5. 1 out of 1 hunk FAILED -- saving rejects to file media_aparat.info.yml.rej patching file src/AparatApi.php Hunk #1 FAILED at 2. Hunk #2 succeeded at 51 with fuzz 1 (offset 8 lines). Hunk #3 succeeded at 92 (offset 19 lines). Hunk #4 succeeded at 114 (offset 19 lines). 1 out of 4 hunks FAILED -- saving rejects to file src/AparatApi.php.rej patching file src/Form/AddVideoForm.php patching file src/Plugin/Field/FieldFormatter/AparatFormatter.php Hunk #6 FAILED at 131. Hunk #7 succeeded at 145 (offset 3 lines). Hunk #8 succeeded at 162 (offset 3 lines). 1 out of 8 hunks FAILED -- saving rejects to file src/Plugin/Field/FieldFormatter/AparatFormatter.php.rej patching file src/Plugin/Field/FieldWidget/AparatWidget.php patching file src/Plugin/media/Source/AparatVideo.php โ media_aparat git:(2.0.x) โ cd .. โ contrib git:(main) โ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig media_aparat FILE: ...mo-site/drupal-orgissue/web/modules/contrib/media_aparat/src/AparatApi.php -------------------------------------------------------------------------------- FOUND 7 ERRORS AND 1 WARNING AFFECTING 7 LINES -------------------------------------------------------------------------------- 7 | ERROR | [x] Use statements should be sorted alphabetically. The first | | wrong one is Drupal\Component\Serialization\Json. 13 | ERROR | [x] Doc comment short description must end with a full stop 14 | WARNING | [ ] The class short comment should describe what the class does | | and not simply repeat the class name 18 | ERROR | [ ] Missing short description in doc comment 23 | ERROR | [ ] Missing short description in doc comment 28 | ERROR | [ ] Missing short description in doc comment 35 | ERROR | [x] Doc comment short description must start with a capital | | letter 35 | ERROR | [x] Doc comment short description must end with a full stop -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: ...s/contrib/media_aparat/src/Plugin/Field/FieldFormatter/AparatFormatter.php -------------------------------------------------------------------------------- FOUND 13 ERRORS AFFECTING 11 LINES -------------------------------------------------------------------------------- 67 | ERROR | [x] The first parameter of a multi-line function declaration | | must be on the line after the opening bracket 68 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 69 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 70 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 71 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 72 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 73 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 74 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 75 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 75 | ERROR | [x] Multi-line function declarations must have a trailing comma | | after the last parameter 75 | ERROR | [x] The closing parenthesis of a multi-line function declaration | | must be on a new line 137 | ERROR | [x] No space found before comment text; expected "// 'width' => | | $max_width ?: $resource->getWidth()," but found "//'width' | | => $max_width ?: $resource->getWidth()," 138 | ERROR | [x] No space found before comment text; expected "// 'height' => | | $max_height ?: $resource->getHeight()," but found | | "//'height' => $max_height ?: $resource->getHeight()," -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 13 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: ...e/web/modules/contrib/media_aparat/src/Plugin/media/Source/AparatVideo.php -------------------------------------------------------------------------------- FOUND 14 ERRORS AND 1 WARNING AFFECTING 13 LINES -------------------------------------------------------------------------------- 103 | ERROR | [x] The first parameter of a multi-line function declaration | | must be on the line after the opening bracket 104 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 105 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 106 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 107 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 108 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 109 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 110 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 111 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 112 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 113 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 114 | ERROR | [x] Multi-line function declaration not indented correctly; | | expected 4 spaces but found 30 114 | ERROR | [x] Multi-line function declarations must have a trailing | | comma after the last parameter 114 | ERROR | [x] The closing parenthesis of a multi-line function | | declaration must be on a new line 122 | WARNING | [ ] The removal-version 'release' does not match the | | lower-case machine-name standard: drupal:n.n.n or | | project:n.x-n.n or project:n.x-n.n-label[n] or | | project:n.n.n or project:n.n.n-label[n] -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 14 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- Time: 276ms; Memory: 10MB
Kindly check
Thanks,
Jake