- Issue created by @ptmkenny
- Status changed to Needs work
over 1 year ago 2:48am 2 May 2023 - 🇵🇭Philippines paraderojether
Hi ptmkenny
I reviewed MR!3, and confirmed it fixes all the issue that you addressed. However I'm getting this new issue/warning that is shown below:
FILE: /Users/studenttrainees/New/drupalorgsite/docroot/modules/contrib/jsonapi_resources/tests/modules/jsonapi_resources_test/jsonapi_resources_test.info.yml
-------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------------------------------
1 | WARNING | "Description" property is missing in the info.yml file
-------------------------------------------------------------------------------------------------------------------------------------------------------------Time: 1.02 secs; Memory: 12MB
Please check.
Thank You. - First commit to issue fork.
- last update
over 1 year ago 7 pass - Status changed to Needs review
over 1 year ago 7:56am 2 May 2023 - 🇮🇳India bharath-kondeti Hyderabad
Updated the MR with the changes. Please review the same.
- Status changed to Needs work
over 1 year ago 8:23am 2 May 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
- // @todo: try to move this exception into Drupal\jsonapi_resources\Routing\ResourceRoutes::ensureResourceImplementationValid(). + // @todo try to move this exception into Drupal\jsonapi_resources\Routing\ResourceRoutes::ensureResourceImplementationValid().
What follows
@todo
is a sentence: It starts with a capitalized word and ends with a period.- $args = $this->argumentResolver->getArguments($request, [$resource, 'process']); + $args = $this->argumentResolver->getArguments( + $request, + [$resource, 'process'], + );
Lines are allowed to be longer than 80 characters, if they are more readable. The existing code is more readable than the changed code.
+ if (!empty(array_intersect_key( + $decoded['data'][0], array_flip(['attributes', 'relationships']) + ))) { throw new UnprocessableEntityHttpException("To add or update a resource object, the request document's primary data must not be an array."); }
As per Drupal coding standards, control structures are written in a single line.
- $route_collection->add('jsonapi_resource_multi_method_route', new Route('/%jsonapi%/resource', $route_defaults, [], [], '', [], ['POST', 'PATCH'])); + $route_collection->add('jsonapi_resource_multi_method_route', new Route( + '/%jsonapi%/resource', + $route_defaults, + [], + [], + '', + [], + ['POST', 'PATCH'], + ));
The existing code is more readable.
- Assigned to nitin_lama
- last update
over 1 year ago 7 pass - Issue was unassigned.
- First commit to issue fork.
- First commit to issue fork.
- 🇮🇳India Shank115
These are remaining issues.
phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml jsonapi_resources/ FILE: /home/sites/jsonapi_resources/src/Unstable/Controller/JsonapiResourceController.php ------------------------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE ------------------------------------------------------------------------------------------------------------------------------------- 66 | ERROR | The array declaration extends to column 82 (the limit is 80). The array content should be split up over multiple lines ------------------------------------------------------------------------------------------------------------------------------------- FILE: /home/sites/jsonapi_resources/src/Unstable/DocumentExtractor.php --------------------------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE --------------------------------------------------------------------------------------------------------------------------------------- 124 | ERROR | The array declaration extends to column 100 (the limit is 80). The array content should be split up over multiple lines --------------------------------------------------------------------------------------------------------------------------------------- FILE: /home/sites/jsonapi_resources/tests/src/Unit/Routing/ResourceRoutesTest.php -------------------------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------------------------------------------------------------- 38 | ERROR | The array declaration extends to column 149 (the limit is 80). The array content should be split up over multiple lines --------------------------------------------------------------------------------------------------------------------------------------
- Assigned to nitin_lama
- 🇮🇳India nitin_lama India
Code is more readable for the remaining issues, no need to fix. Lets see what the maintainer says.
- Issue was unassigned.
- Status changed to Needs review
10 months ago 7:25am 27 February 2024 - First commit to issue fork.
- 🇺🇸United States mglaman WI, USA
There are two MRs. That's confusing. I rebased both to have GitLab CI running and determine which is most correct.
-
mglaman →
committed c0abb9a8 on 8.x-1.x authored by
ptmkenny →
Issue #3339626 by mglaman, nitin_lama, ptmkenny, Shank115, bharath-...
-
mglaman →
committed c0abb9a8 on 8.x-1.x authored by
ptmkenny →
- Status changed to Fixed
9 months ago 1:56pm 19 March 2024 Automatically closed - issue fixed for 2 weeks with no activity.