Problem/Motivation

Because we have a version of XdebugValidator in package_manager and automatic_updates we have the same message 2x on the status report

Steps to reproduce

Proposed resolution

TBD but we should only have the message 1x.

Might be easiest if the automatic_updates to just replaces the class for the validator service.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#2 Screen Shot 2022-12-08 at 10.56.42 PM.png39.26 KBtedbow

Issue fork automatic_updates-3321206

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

tedbow created an issue. See original summary.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Issue summary: View changes
Issue tags: +sprint
StatusFileSize
new Screen Shot 2022-12-08 at 10.56.42 PM.png39.26 KB
omkar.podey’s picture

omkar.podey commented
Assigned: Unassigned » omkar.podey

omkar.podey opened merge request !620

omkar.podey’s picture

omkar.podey commented

The MR above is expected to fail, i have a fix commented out in \Drupal\package_manager\Validator\XdebugValidator::checkForXdebug because xdebug validator in package manager adds an additional warning everytime.

We can test that fix via \Drupal\Tests\automatic_updates\Functional\StatusCheckTest::testStatusChecksOnAdminPages which has a file_put_contents which needs to be updated accordingly, now if we want to test it we need to enable both xdebug validators somehow either for that particular test or have another base class for it, Thoughts?

omkar.podey closed merge request !620

omkar.podey opened merge request !622

omkar.podey’s picture

omkar.podey commented

Still needs test coverage, so is there a way to enable disabled validators in functional tests ?, I removed all the logic I felt unecessary and just left of what I expected the Validator to do, I have verified with the implementation that both the duplicating cases are eliminated with it.

omkar.podey’s picture

omkar.podey commented
Assigned: omkar.podey » wim leers
Status: Active » Needs review
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » omkar.podey
Status: Needs review » Needs work

#8: I think you've answered this yourself now! 👍😊

5 remarks on the MR.

omkar.podey’s picture

omkar.podey commented
Assigned: omkar.podey » Unassigned
Status: Needs work » Needs review
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

🚀

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Reviewed & tested by the community » Needs work

This is so much better than 8.x-2.x!

Needs Work for 1 MR comment

omkar.podey’s picture

omkar.podey commented
Assigned: Unassigned » omkar.podey
omkar.podey’s picture

omkar.podey commented
Assigned: omkar.podey » Unassigned
Status: Needs work » Needs review
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

👍

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Issue tags: +core-mvp
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: Unassigned » tedbow

I pushed a couple changes. will commit if green

thanks!

  • tedbow committed 8a988af2 on 8.x-2.x authored by omkar.podey 
    Issue #3321206 by omkar.podey, tedbow, Wim Leers: The same xdebug...
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » Unassigned
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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