Skip to content
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

Ruleset::getIncludePatterns(): add tests #706

Merged
merged 2 commits into from
Nov 22, 2024

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Nov 21, 2024

Suggested changelog entry

N/A

Related issues/external references

This PR is part of a series of PRs expanding the tests for the Ruleset class.

Copy link
Contributor

@rodrigoprimo rodrigoprimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I just have the same comments that I left for #705. Should the test contain include patterns with the type attribute? Both with absolute (the default value) and relative.

As a sidenote not directly related to this PR, it seems PHPCS doesn't validate the value of the type attribute, and users can pass any value. Maybe there should be some validation? Happy to create a separate issue for this if you agree.

@jrfnl
Copy link
Member Author

jrfnl commented Nov 22, 2024

See my response to these same points in #705 (comment)

@jrfnl jrfnl merged commit a480170 into master Nov 22, 2024
70 checks passed
@jrfnl jrfnl deleted the feature/ruleset-add-tests-getincludepatterns branch November 22, 2024 19:21
jrfnl added a commit that referenced this pull request Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants