Skip to content

[6.x] Remove redundant default attributes from phpunit.xml #5233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 24, 2020
Merged

[6.x] Remove redundant default attributes from phpunit.xml #5233

merged 1 commit into from
Feb 24, 2020

Conversation

SjorsO
Copy link
Contributor

@SjorsO SjorsO commented Feb 23, 2020

PR #5139 added the xml schema to phpunit.xml. Because of this, IDEs like PhpStorm now show which attributes are redundant:
image

All the gray attributes in the screenshot above are setting the same value as the default value. They can be removed.

The suffix="Test.php" is also a redundant default, but I think this is useful in reminding people their test classes should end with Test.php

@GrahamCampbell
Copy link
Member

Kinda stupid IDEs would warn about this. We included these options so people don't have to head to Google to know what the options are called to change them.

@GrahamCampbell GrahamCampbell changed the title Remove redundant default attributes from phpunit.xml [6.x] Remove redundant default attributes from phpunit.xml Feb 23, 2020
@SjorsO
Copy link
Contributor Author

SjorsO commented Feb 23, 2020

Kinda stupid IDEs would warn about this. We included these options so people don't have to head to Google to know what the options are called to change them.

I've never had to change any of these values, I actually don't know what most of these are supposed to do. Removing them would clean up some unnecessary noise in this file.

@GrahamCampbell
Copy link
Member

Maybe keep stopOnFailure in there, at least?

@taylorotwell taylorotwell merged commit 8cece72 into laravel:master Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants