CommentFileSizeAuthor
#6 interdiff_2-6.txt846 bytesleolandotan
#6 3112641-6.patch846 bytesleolandotan
#2 3112641-2.patch6.26 KBhardik_patel_12

Comments

Hardik_Patel_12 created an issue. See original summary.

hardik_patel_12’s picture

hardik_patel_12 commented
StatusFileSize
new 3112641-2.patch6.26 KB

Kindly review a patch.

hardik_patel_12’s picture

hardik_patel_12 commented
Assigned: hardik_patel_12 » Unassigned
Status: Active » Needs review
hash6’s picture

hash6 commented
Assigned: Unassigned » hash6
xjm’s picture

xjm
she/her
Primary language English
commented
Title: Remove all @deprecated code from datetime_range module » Remove stale reference to datetime_range_view_presave()
Status: Needs review » Needs work

datetime_range_view_presave() was already removed in a different issue. However, the other issue didn't fix the @see (the second hunk of this patch). So, can we create a new patch with only the second hunk? Thanks!

leolandotan’s picture

leolandotan commented
Status: Needs work » Needs review
StatusFileSize
new 3112641-6.patch846 bytes
new interdiff_2-6.txt846 bytes

Hi! I went ahead and updated the patch by doing a re-roll because the patch in #2 didn't cleanly apply to the latest 9.0.x code base anymore.

I tested the patch using `➜ drupal git:(9.0.x) ✗ git apply --check 3112641-6.patch` and there was no output so I think this is alright.

I just went and added an interdiff even if the path and the interdiff looks the same.

Hope everything is in order.

Thank you!

Status: Needs review » Needs work

The last submitted patch, 6: 3112641-6.patch, failed testing. View results

sja112’s picture

sja112 commented

Adding patch shared in #6 for retesting.

I have added this patch to my local and tested it after applying the patch. For me, this test case worked in local.

Local drupal version: 9.0.x
Local PHPUnit version: 8.5.4
PHP version: 7.3.17

root@3b1a1f78358c:/var/www/html# ./vendor/bin/phpunit -c core/phpunit.xml core/modules/media_library/tests/src/FunctionalJavascript/WidgetUploadTest.php
PHPUnit 8.5.4 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\media_library\FunctionalJavascript\WidgetUploadTest
SS                                                                  2 / 2 (100%)

Time: 4.11 minutes, Memory: 8.00 MB

OK, but incomplete, skipped, or risky tests!
Tests: 2, Assertions: 2, Skipped: 2.

Thanks,
Sandesh Jain.

sja112’s picture

sja112 commented
Assigned: hash6 » sja112
Status: Needs work » Needs review
sja112’s picture

sja112 commented
Status: Needs review » Reviewed & tested by the community

This patch applied smoothly and passed all the test cases. So, moving this to Reviewed and Tested by the community.

sja112’s picture

sja112 commented
Assigned: sja112 » Unassigned

  • xjm committed 95ff62a on 9.1.x 
    Issue #3112641 by leolando.tan, Hardik_Patel_12, xjm: Remove stale...

  • xjm committed 08c39a3 on 9.0.x 
    Issue #3112641 by leolando.tan, Hardik_Patel_12, xjm: Remove stale...
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Thanks for working on this!

@sja112, The automated testing infrastructure tells us whether the patch applies and passes tests, so we do not need people to review that. What we need people to review is whether the change is correct and complete. :)

In this case, the docs referred to a removed hook implementation for which there was no direct replacement, so simply removing the stale docs is sufficient to resolve the issue.

Committed to 9.1.x and 9.0.x. Thanks!

Status: Fixed » Closed (fixed)

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