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

twigcs exclude subfolders #686

Merged
merged 4 commits into from
Mar 19, 2020
Merged

Conversation

oallain
Copy link
Contributor

@oallain oallain commented Oct 10, 2019

Q A
Branch master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Documented? yes
Fixed tickets #657 (comment)

Add exclude option on TwigCs after add it in friendsoftwig/twigcs#81

I think we have to wait for the next Tag on friendsoftwig/twigcs before we merge this PR, because the feature is only on Master branch currently 😉 (See friendsoftwig/twigcs#82)

src/Task/TwigCs.php Outdated Show resolved Hide resolved
src/Task/TwigCs.php Outdated Show resolved Hide resolved
src/Task/TwigCs.php Outdated Show resolved Hide resolved
@veewee
Copy link
Contributor

veewee commented Oct 10, 2019

@oallain Thanks for the PR!
We'll wait untill it is released. In the meantime I've added some small improvements on the code.

src/Task/TwigCs.php Outdated Show resolved Hide resolved
@oallain oallain force-pushed the feature/twigcs-exclude branch from d906c65 to 0a5dd6f Compare October 11, 2019 11:06
Copy link
Contributor

@veewee veewee left a comment

Choose a reason for hiding this comment

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

Looks good @oallain
Let me know once it is available in a stable version of twigcs.

@Landerstraeten Landerstraeten self-requested a review November 29, 2019 09:13
@truls1502
Copy link
Contributor

Will a beta version friendsoftwig/twigcs#82 (comment) be okay or do need to use a stable version? Since I have used patch until now and it works fine.

@oallain oallain force-pushed the feature/twigcs-exclude branch from 0a5dd6f to 99ce3e1 Compare March 18, 2020 21:15
@oallain
Copy link
Contributor Author

oallain commented Mar 18, 2020

@truls1502 thanks for feedback :)


I rebase this PR and update it with new Task tests with PHPUnit.
It works fine, but i have an error on AppVeyor, which i don't understand (and not on my code changes) :

  1. GrumPHPTest\Unit\Linter\Xml\XmlLinterTest::it_should_validate_xml_with_dtd with data set Windows support #5 ('dtd-url-valid.xml', 0, true)
    DOMDocument::load(): php_network_getaddresses: getaddrinfo failed: No such host is known.

Anyone know why?

@veewee veewee added this to the 0.18.1 milestone Mar 19, 2020
@veewee veewee merged commit b835400 into phpro:master Mar 19, 2020
@veewee
Copy link
Contributor

veewee commented Mar 19, 2020

Thanks for rebasing! Looks like we can merge it in and still keep BC to the older version of the tool, so good to go IMO.

@oallain oallain deleted the feature/twigcs-exclude branch March 19, 2020 18:28
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.

4 participants