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

Add attribute reader #32

Merged
merged 5 commits into from
Mar 14, 2023
Merged

Add attribute reader #32

merged 5 commits into from
Mar 14, 2023

Conversation

icanhazstring
Copy link
Contributor

Solves #25

This is a basic straight forward implementation to support attributes.
The attributes are provided by this package, as we can't rely on PHPUnit attribute parser implementation which is tied to its own attributes.

@jakzal
Copy link
Owner

jakzal commented Feb 14, 2023

Amazing @icanhazstring ! Thanks for this. I'll take a look as soon as I can.

@icanhazstring
Copy link
Contributor Author

Sure thing. Just wanted to upgrade to PHPUnit 10 and saw that open issue 😉

Will fix the code sniffer alerts later in.
If you have anything to be changed just say so, will happily change it.

@icanhazstring
Copy link
Contributor Author

icanhazstring commented Feb 16, 2023

Thinking about it. Maybe we should go with a new AttributeExtension instead of adding attribute parsing into AnnotationExtension.
So we are separate and could drop the old ins some future release if wanted.

README.md Show resolved Hide resolved
@jakzal
Copy link
Owner

jakzal commented Feb 18, 2023

Looks good!

Thinking about it. Maybe we should go with a new AttributeExtension instead of adding attribute parsing into AnnotationExtension.

Makes sense. We should also document the migration path.

So we are separate and could drop the old ins some future release if wanted.

Yep, that's the plan 👍

@icanhazstring
Copy link
Contributor Author

Thanks for the feedback. Yes it was pretty quickly put together 😉

Will take some time to improve the PR 👍

@jakzal
Copy link
Owner

jakzal commented Mar 13, 2023

@icanhazstring sorry for the radio silence. Is this ready for another review?

@icanhazstring
Copy link
Contributor Author

No worries. Yes go ahead 🙂

Copy link
Owner

@jakzal jakzal left a comment

Choose a reason for hiding this comment

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

Great work. Nice use of FP techniques 👍

src/Attribute/TargetAware.php Outdated Show resolved Hide resolved
src/GlobalsAttributeReader.php Outdated Show resolved Hide resolved
phpunit.xml.dist Show resolved Hide resolved
src/GlobalsAttributeReader.php Show resolved Hide resolved
@icanhazstring
Copy link
Contributor Author

Thanks. A pleasure 🙂

@jakzal
Copy link
Owner

jakzal commented Mar 14, 2023

Superb work @icanhazstring. Thank you 🍺

@jakzal jakzal merged commit a357c80 into jakzal:main Mar 14, 2023
This was referenced Mar 14, 2023
@jakzal
Copy link
Owner

jakzal commented Mar 14, 2023

Released as 3.1.0.

@icanhazstring icanhazstring deleted the feature/attribute-reader branch March 14, 2023 16:15
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.

None yet

2 participants