- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5
362 improve code #145
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
362 improve code #145
Conversation
| Reviewer's GuideThis pull request enhances the project skeleton by conditionally injecting Symfony Mailer configuration into environment files, standardizing CI scripts to invoke vendor binaries through the PHP executable, and updating development dependencies in composer.json. File-Level Changes
 Tips and commandsInteracting with Sourcery
 Customizing Your ExperienceAccess your dashboard to: 
 Getting Help
 | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @tijsverkoyen - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/Skeleton/PostCreateProject.php:286` </location>
<code_context>
+        preg_match('|###< symfony/mailer ###|mU', $content, $matches, PREG_OFFSET_CAPTURE);
+        if(!empty($matches)) {
+            $offset = $matches[0][1];
+            unset($insert[0]);
+            unset($insert[4]);
+        }
+        $content = self::insertStringAtPosition(
</code_context>
<issue_to_address>
Unsetting array elements by index may be fragile if the insert array changes.
Using hardcoded indices (0 and 4) makes the code brittle if the structure of $insert changes. Prefer array_shift, array_pop, or referencing elements by value for better maintainability.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
        if(!empty($matches)) {
            $offset = $matches[0][1];
            unset($insert[0]);
            unset($insert[4]);
        }
=======
        if(!empty($matches)) {
            $offset = $matches[0][1];
            array_shift($insert); // Remove the first element
            array_pop($insert);   // Remove the last element
        }
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances mailer configuration in the project skeleton, upgrades PHPUnit settings to v12 standards, and adjusts CI and dev dependencies accordingly.
- Add symfony/mailermarkers to.envand.env.localinPostCreateProject.php
- Migrate phpunit.xml.distto the PHPUnit 12 XSD, enable stricter output/deprecation settings, and remove legacy listeners
- Update dev dependencies and CI commands: add DoctrineTestBundle & PHPUnit 12, remove symfony/phpunit-bridge, prefix vendor/bincalls withphp; setMAILER_DSNin.env.test
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description | 
|---|---|
| src/Skeleton/PostCreateProject.php | add logic to insert mailer environment markers | 
| phpunit.xml.dist | update PHPUnit schema, add strict settings & extensions | 
| composer.json | bump dev deps ( dama/doctrine-test-bundle,phpunit/phpunit), remove bridge | 
| .gitlab-ci.yml | change CI scripts to call php vendor/bin/... | 
| .env.test | set default MAILER_DSNfor tests | 
Comments suppressed due to low confidence (2)
.gitlab-ci.yml:217
- Vendor binaries often include a shebang and are meant to be executed directly; prefixing the deploy script with php can fail if it's a shell script. Consider calling vendor/bin/dep directly.
    - php vendor/bin/dep deploy stage=staging
phpunit.xml.dist:12
- Within the section, the element should be (not ) when registering a PHPUnit extension class according to the phpunit.xsd.
        <bootstrap class="DAMA\DoctrineTestBundle\PHPUnit\PHPUnitExtension" />
https://next-app.activecollab.com/108877/my-work?modal=Task-224038-533
Summary by Sourcery
Scaffold Symfony Mailer configuration in .env files, update development dependencies, and standardize CI scripts to invoke PHP-prefixed vendor binaries
New Features:
Build:
CI: