1. 8 according to JSON API's testing: https://www.drupal.org/pift-ci-job/919614 — these are regressions that we let slip in
  2. 13 according to core's testing: https://www.drupal.org/pift-ci-job/925827 — most of these are due to core apparently having certain coding standards that don't exist in the Drupal coding standard

Discovered in #2843147-51: Add JSON:API to core as a stable module.

CommentFileSizeAuthor
#12 2957274-12.patch1.49 KBwim leers
#8 2957274-8.patch14.51 KBwim leers
#6 2957274-6.patch15.48 KBwim leers
#6 interdiff.txt1.73 KBwim leers
#4 2957274-4.patch15.03 KBwim leers
#4 interdiff.txt10.76 KBwim leers
#3 2957274-3.patch4.65 KBwim leers
#3 interdiff.txt311 byteswim leers
#2 2957274-2.patch4.32 KBwim leers

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new 2957274-2.patch4.32 KB
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
StatusFileSize
new interdiff.txt311 bytes
new 2957274-3.patch4.65 KB

most of these are due to core apparently having certain coding standards that don't exist in the Drupal coding standard

So let's expand JSON API's coding standards to include those!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
StatusFileSize
new interdiff.txt10.76 KB
new 2957274-4.patch15.03 KB

This should make it all green again.

Status: Needs review » Needs work

The last submitted patch, 4: 2957274-4.patch, failed testing. View results

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.73 KB
new 2957274-6.patch15.48 KB

Hah, turns out that testRasterizeValueFails() was wrong/broken/testing the wrong thing. That's exactly why core mandates the use of setExpectedException()! Hurray for coding standards helping to ensure more reliable tests.

gabesullice’s picture

gabesullice
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Nice work!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
StatusFileSize
new 2957274-8.patch14.51 KB

RequestHandlerTest no longer exists and EntityResourceTest was changed. Rebased, retesting.

  • gabesullice committed 54b5f5f on 8.x-2.x 
    Issue #2957274 by Wim Leers, gabesullice: Fix coding standard violations...

  • gabesullice committed 0d338c1 on 8.x-1.x 
    Issue #2957274 by Wim Leers, gabesullice: Fix coding standard violations...
gabesullice’s picture

gabesullice
Primary language English
commented
Version: 8.x-1.x-dev » 8.x-2.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: +API-First Initiative

Thanks! Committed and cherry-picked to 1.x.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
StatusFileSize
new 2957274-12.patch1.49 KB

Thanks! But there still were some other CS violations. Due to other recent commits. No big deal though :) Want to fix them here or elsewhere?

  • gabesullice committed 149f13a on 8.x-2.x 
    Issue #2957274 by Wim Leers, gabesullice: Fix coding standard violations...

  • gabesullice committed c94819a on 8.x-1.x 
    Issue #2957274 by Wim Leers, gabesullice: Fix coding standard violations...
gabesullice’s picture

gabesullice
Primary language English
commented

Whoops, done.

Status: Fixed » Closed (fixed)

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