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.

Sign up for GitHub

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

Jump to bottom

Add twig extension #36

Merged
merged 1 commit into from Oct 25, 2022
Merged

Add twig extension #36

merged 1 commit into from Oct 25, 2022

Conversation

ajgarlag
Copy link
Contributor

Closes #32

@codecov
Copy link

codecov bot commented Oct 20, 2022 •
edited

Codecov Report

Merging #36 (f2c4377) into 1.0.x (4f89019) will not change coverage.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##               1.0.x       #36   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity        20        28    +8     
===========================================
  Files              8        11    +3     
  Lines            174       193   +19     
===========================================
+ Hits             174       193   +19
Impacted Files Coverage Δ
src/DependencyInjection/TwigExtensionPass.php 100.00% <100.00%> (ø)
src/PheatureFlagsBundle.php 100.00% <100.00%> (ø)
src/Twig/PheatureFlagsExtension.php 100.00% <100.00%> (ø)
src/Twig/PheatureFlagsRuntime.php 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kpicaza kpicaza added the enhancement New feature or request label Oct 20, 2022
kpicaza
kpicaza reviewed Oct 20, 2022
View reviewed changes
Copy link
Member

@kpicaza kpicaza left a comment

Choose a reason for hiding this comment

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

Thanks @ajgarlag for the excellent work here 👏👏👏 I added a change suggestion about PHPUnit API usage.

I will create a doc page based on the issue of how to use the Pheature Flags Twig Extension

PD: If you want don't worry about Codecov messages, at this point is not our priority to have 100% test coverage, also we can rework them to apply the integration test approach we are using in other compiler passes.

PD2: If you have any questions, feedback, or so on, you are welcome to ask. Thanks again for contributing 🙌🙌🙌 .

test/Twig/PheatureFlagsExtensionTest.php Outdated
public function testItShouldExposeTwoTests(): void
{
$extension = new PheatureFlagsExtension(new Toggle($this->createStub(FeatureFinder::class)));
self::assertCount(2, $extension->getTests());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self::assertCount(2, $extension->getTests());
$this->assertCount(2, $extension->getTests());

We decide some time ago to use always $this to maintain the same syntax in all PHPUnit API usage pheature-flags/toggle-core#49

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

src/DependencyInjection/TwigExtensionPass.php Show resolved Hide resolved
src/PheatureFlagsBundle.php Show resolved Hide resolved
@kpicaza
Copy link
Member

kpicaza commented Oct 24, 2022

Hello @ajgarlag, I merged #35 adding test cases for the Bundle class. If you want I can make a Pull request to your fork fixing the issues commented above.

Thank you very much for contributing

@ajgarlag ajgarlag force-pushed the feature/twig-extension branch 2 times, most recently from 0b63e99 to 9b2c9c1 Compare October 25, 2022 06:30
@ajgarlag ajgarlag marked this pull request as ready for review October 25, 2022 06:32
@ajgarlag
Copy link
Contributor Author

Hello @ajgarlag, I merged #35 adding test cases for the Bundle class. If you want I can make a Pull request to your fork fixing the issues commented above.

Thank you very much for contributing

Hi, @kpicaza, I think all issues have been addressed and the PR is ready.

@sonarcloud
Copy link

sonarcloud bot commented Oct 25, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@kpicaza kpicaza self-requested a review October 25, 2022 15:52
kpicaza
kpicaza approved these changes Oct 25, 2022
View reviewed changes
Copy link
Member

@kpicaza kpicaza left a comment

Choose a reason for hiding this comment

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

Nice job!!! Ready to release 🚢🚢🚢

@kpicaza kpicaza merged commit 579bd4b into pheature-flags:1.0.x Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kpicaza kpicaza kpicaza approved these changes

@xserrat xserrat Awaiting requested review from xserrat

Assignees

@ajgarlag ajgarlag

@kpicaza kpicaza

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[symfony-toggle] Twig extension
2 participants
@ajgarlag @kpicaza