Cleanup duplicate requirements in simpletest_requirements()

CommentFileSizeAuthor
#23 dom_and_hash_extensions-895656-22.patch1.34 KBadinac
#21 dom_and_hash_extensions-895656-21.patch1.37 KBadinac
#19 895656-19.patch1.44 KBandypost
#7 d8-simpletest_requirements-895656-7.patch1.47 KBsmccabe
#3 d7-simpletest_requirements-895656-3.patch1.94 KBsmccabe
simpletest-requirements.patch2.1 KBmfb

Comments

aspilicious’s picture

aspilicious commented

Well....

If you're removing some code from testing module and bot gives green light than this must be rtbc...

or not?

thedavidmeister’s picture

thedavidmeister commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Novice, +Needs reroll

Seems fair. I think with a reroll this is RTBC.

smccabe’s picture

smccabe commented
StatusFileSize
new d7-simpletest_requirements-895656-3.patch1.94 KB

Rerolled

smccabe’s picture

smccabe commented
Status: Needs work » Needs review
travis-bradbury’s picture

travis-bradbury commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

To review this patch I:

  1. Verified simpletest installs and runs successfully on 7.x-dev
  2. Applied the patch
  3. Verified that simpletest still installs and runs successfully
David_Rothstein’s picture

David_Rothstein commented
Version: 7.x-dev » 8.0.x-dev
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs backport to D7

Looks like this would need to go into Drupal 8 first - at least the DOMDocument part of it is still there.

smccabe’s picture

smccabe commented
StatusFileSize
new d8-simpletest_requirements-895656-7.patch1.47 KB

Rerolled for 8

smccabe’s picture

smccabe commented
Status: Needs work » Needs review
snehi’s picture

snehi commented

Looks ok to me. Should be gone for RTBC.

valthebald’s picture

valthebald
Location Sofia
commented
Status: Needs review » Reviewed & tested by the community

Removing code, and it doesn't break tests, good stuff!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: d8-simpletest_requirements-895656-7.patch, failed testing.

sdstyles’s picture

sdstyles commented
Status: Needs work » Needs review
sdstyles’s picture

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

Retested patch and it didn't fail so it's RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: d8-simpletest_requirements-895656-7.patch, failed testing.

snehi’s picture

snehi commented
Status: Needs work » Reviewed & tested by the community

Setting Back to RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: d8-simpletest_requirements-895656-7.patch, failed testing.

snehi’s picture

snehi commented
Status: Needs work » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: d8-simpletest_requirements-895656-7.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Version: 8.0.x-dev » 8.1.x-dev
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new 895656-19.patch1.44 KB

Re-roll for 8.1

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 19: 895656-19.patch, failed testing.

adinac’s picture

adinac commented
Status: Needs work » Needs review
Issue tags: +SprintWeekend2016, +SprintWeekendTM
StatusFileSize
new dom_and_hash_extensions-895656-21.patch1.37 KB

Status: Needs review » Needs work

The last submitted patch, 21: dom_and_hash_extensions-895656-21.patch, failed testing.

adinac’s picture

adinac commented
Status: Needs work » Needs review
StatusFileSize
new dom_and_hash_extensions-895656-22.patch1.34 KB
andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

this change is straight so some bot flux should not stop that

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 8.1.x-dev » 7.x-dev
Category: Bug report » Task
Status: Reviewed & tested by the community » Patch (to be ported)

Yep system_requirements() makes the dom extension mandatory so this does indeed look superfluous. However I thought this might be tricky because we have unit tests that rely on the dom extension - but actually phpunit requires it too... https://packagist.org/packages/phpunit/phpunit.

Committed 0146f0f and pushed to 8.1.x. Thanks! I've committed this to 8.1.x because I don't think it is a bug - yes double checking is pointless but nothing is broken.

  • alexpott committed 0146f0f on 8.1.x 
    Issue #895656 by smccabe, adinac, andypost, mfb: dom and hash extensions...

  • alexpott committed 0146f0f on 8.3.x 
    Issue #895656 by smccabe, adinac, andypost, mfb: dom and hash extensions...

  • alexpott committed 0146f0f on 8.3.x 
    Issue #895656 by smccabe, adinac, andypost, mfb: dom and hash extensions...
stefan.r’s picture

stefan.r commented
Issue tags: +Dublin2016

Marking the backport to D7 as a novice task

  • alexpott committed 0146f0f on 8.4.x 
    Issue #895656 by smccabe, adinac, andypost, mfb: dom and hash extensions...

  • alexpott committed 0146f0f on 8.4.x 
    Issue #895656 by smccabe, adinac, andypost, mfb: dom and hash extensions...

Status: Patch (to be ported) » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.