Conversation
mesilov
commented
Mar 3, 2026
| Q | A |
|---|---|
| Bug fix? | yes/no |
| New feature? | no |
| Deprecations? | no |
| Issues | Fix #84... |
| License | MIT |
…v3` style; remove legacy targets, standardize commands, and introduce grouped help sections. Signed-off-by: mesilov <mesilov.maxim@gmail.com>
…tter compatibility, including `b24phpsdk 3.0.*`, `doctrine-bundle 3.2.2`, `migrations-bundle 4.0.0`, and allow Symfony components ^7||^8 Signed-off-by: mesilov <mesilov.maxim@gmail.com>
…/property-access` to `require-dev` and update dependencies. Signed-off-by: mesilov <mesilov.maxim@gmail.com>
… align with `ContactPersonInterface`, implement `isPartner()`, and ensure compatibility with SDK contracts. Signed-off-by: mesilov <mesilov.maxim@gmail.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's maintainability and stability by addressing several key areas. It modernizes the Makefile for a more consistent development experience, resolves critical test failures by bringing entity interfaces in line with the SDK, and updates dependencies to ensure compatibility and remove deprecation warnings. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request introduces several important updates, primarily bumping the bitrix24/b24phpsdk to version 3.0.* and ensuring compatibility. This includes updating the ContactPerson entity to align with the SDK's ContactPersonInterface by adjusting method signatures and adding a missing method. The Makefile has been refactored to adopt the b24phpsdk v3 style, improving consistency and usability. Additionally, a missing symfony/property-access dependency has been added to require-dev to resolve unit test failures. The CHANGELOG.md accurately reflects these changes.
| @echo "remove all containers" | ||
| docker-compose down --remove-orphans | ||
| docker compose down --remove-orphans | ||
| @echo "pull Docker images" | ||
| docker compose pull | ||
| @echo "build containers" | ||
| docker-compose build | ||
| docker compose build | ||
| @echo "install dependencies" | ||
| docker-compose run --rm php-cli composer install | ||
| @echo "change owner of var folder for access from container" | ||
| docker-compose run --rm php-cli chown -R www-data:www-data /var/www/html/var/ | ||
| @echo "run application…" | ||
| docker-compose up -d | ||
|
|
||
| docker compose run --rm php-cli composer install | ||
| @echo "run application..." | ||
| docker compose up -d | ||
|
|
There was a problem hiding this comment.
The docker-init target no longer includes the chown -R www-data:www-data /var/www/html/var/ command. This command was previously responsible for setting correct permissions for the var directory within the container. Please ensure that this permission setting is now handled by the Docker image itself or through volume configurations, otherwise, its absence could lead to permission-related issues at runtime.
…onfigurations to target PHP 8.4 only. Signed-off-by: mesilov <mesilov.maxim@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c09f015c7b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .PHONY: docker-up | ||
| docker-up: |
There was a problem hiding this comment.
Add aliases for removed Make targets
This renaming drops previously documented targets like make up and make test-run-functional (see README.md:99-102) without compatibility aliases, and because the wildcard rule in Makefile:14-15 swallows unknown goals, those old commands now exit successfully as no-ops instead of running setup/tests. In practice, users following the existing quick-start flow will think services/tests ran when nothing happened, which is a functional regression in developer workflow.
Useful? React with 👍 / 👎.
| "doctrine/doctrine-bundle": "3.2.2", | ||
| "doctrine/doctrine-migrations-bundle": "4.0.0", |
There was a problem hiding this comment.
Replace exact Doctrine bundle pins with semver ranges
Pinning doctrine/doctrine-bundle and doctrine/doctrine-migrations-bundle to exact versions makes this library unnecessarily conflict with consumers that already use compatible newer patch/minor releases, reducing installability across projects; composer validate --strict also flags these exact constraints as problematic for semver packages. Using bounded ranges avoids turning routine dependency updates into hard conflicts.
Useful? React with 👍 / 👎.
…name with `Uuid` alias. Signed-off-by: mesilov <mesilov.maxim@gmail.com>
… build/publish images, integrate GHCR usage in tests, and update local dev setup. Signed-off-by: mesilov <mesilov.maxim@gmail.com>
…lows, update `docker-compose.yaml` to use registry image with fallback to local build, and enhance PostgreSQL setup in functional tests. Signed-off-by: mesilov <mesilov.maxim@gmail.com>
Signed-off-by: mesilov <mesilov.maxim@gmail.com>
…phpsdk v3` style Signed-off-by: mesilov <mesilov.maxim@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7936f7a429
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| docker-compose down --remove-orphans | ||
| docker compose down --remove-orphans | ||
| @echo "pull Docker images" | ||
| docker compose pull |
There was a problem hiding this comment.
Allow docker-init to continue when registry pull fails
docker-init now executes docker compose pull before docker compose build, and Make stops on the first non-zero command. When the GHCR tag is unavailable or inaccessible (for example, fresh/forked environments or transient registry issues), initialization aborts before the local build step, so the compose build fallback path is never reached and first-time setup fails.
Useful? React with 👍 / 👎.
| contents: read | ||
| packages: read | ||
| container: | ||
| image: ghcr.io/mesilov/bitrix24-php-lib:php-cli |
There was a problem hiding this comment.
Restore CI coverage for declared PHP 8.3 support
This workflow now always runs inside the php-cli container image (built from php:8.4), while composer.json still declares "php": "8.3.* || 8.4.*". Because the previous 8.3 matrix coverage was removed (same pattern in functional tests), PHP 8.3-specific regressions can now be merged without detection, which breaks alignment between CI guarantees and the published platform constraint.
Useful? React with 👍 / 👎.