-
Notifications
You must be signed in to change notification settings - Fork 5
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
Added RR_VERSION env to Environment class #37
Conversation
Signed-off-by: Plakhotnikov Vladimir <v.plahotnikov@yclients.tech>
WalkthroughThe recent updates introduce a new method Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
src/EnvironmentInterface.php (1)
Line range hint
16-16
: Resolve missing imports and attributes.- use JetBrains\PhpStorm\ExpectedValues; - use Spiral\RoadRunner\Environment\Mode; + // Ensure these classes exist or correct the import paths.Also applies to: 23-23
src/Environment.php (1)
Line range hint
19-19
: Resolve missing imports and incorrect interface implementation.- use Spiral\RoadRunner\Environment\Mode; - class Environment implements EnvironmentInterface + // Ensure the class `Mode` exists or correct the import path. + // Verify that `Environment` correctly implements `EnvironmentInterface`.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/Environment.php (2 hunks)
- src/EnvironmentInterface.php (1 hunks)
Additional context used
PHPStan
src/EnvironmentInterface.php
16-16: Cannot import type alias ModeType: class Spiral\RoadRunner\Environment\Mode does not exist.
23-23: Attribute class JetBrains\PhpStorm\ExpectedValues does not exist.
23-23: Class Spiral\RoadRunner\Environment\Mode not found.
src/Environment.php
19-19: Cannot import type alias ModeType: class Spiral\RoadRunner\Environment\Mode does not exist.
19-19: Class Spiral\RoadRunner\Environment implements unknown interface Spiral\RoadRunner\EnvironmentInterface.
Additional comments not posted (2)
src/EnvironmentInterface.php (1)
36-39
: The methodgetVersion()
is correctly added as per the PR objectives.src/Environment.php (1)
44-47
: The methodgetVersion()
is correctly added as per the PR objectives.
Signed-off-by: Plakhotnikov Vladimir <v.plahotnikov@yclients.tech>
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tests/Unit/EnvironmentTest.php (1 hunks)
Additional context used
PHPStan
tests/Unit/EnvironmentTest.php
10-10: Class Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest extends unknown class PHPUnit\Framework\TestCase.
14-14: Instantiated class Spiral\RoadRunner\Environment not found.
15-15: Call to an undefined method Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest::assertEquals().
20-20: Instantiated class Spiral\RoadRunner\Environment not found.
21-21: Call to an undefined method Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest::assertEquals().
26-26: Instantiated class Spiral\RoadRunner\Environment not found.
27-27: Call to an undefined method Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest::assertEquals().
32-32: Instantiated class Spiral\RoadRunner\Environment not found.
33-33: Call to an undefined method Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest::assertEquals().
38-38: Instantiated class Spiral\RoadRunner\Environment not found.
39-39: Call to an undefined method Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest::assertEquals().
44-44: Instantiated class Spiral\RoadRunner\Environment not found.
45-45: Call to an undefined method Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest::assertEquals().
50-50: Instantiated class Spiral\RoadRunner\Environment not found.
51-51: Call to an undefined method Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest::assertEquals().
60-60: Call to static method fromGlobals() on an unknown class Spiral\RoadRunner\Environment.
62-62: Call to an undefined method Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest::assertEquals().
63-63: Call to an undefined method Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest::assertEquals().
64-64: Call to an undefined method Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest::assertEquals().
65-65: Call to an undefined method Spiral\RoadRunner\Tests\Worker\Unit\EnvironmentTest::assertEquals().
Additional comments not posted (3)
tests/Unit/EnvironmentTest.php (3)
48-52
: LGTM! The testtestGetVersionWithValue
correctly sets and asserts theRR_VERSION
environment variable.
58-64
: LGTM! The testtestFromGlobals
now includes setting and asserting theRR_VERSION
environment variable, ensuring it works with global environment settings.
Line range hint
10-10
: Please verify the setup of your static analysis tool to ensure it recognizes all project dependencies and paths. This will help avoid false positives regarding missing classes and methods.Also applies to: 14-14, 15-15, 20-20, 21-21, 26-26, 27-27, 32-32, 33-33, 38-38, 39-39, 44-44, 45-45, 50-50, 51-51, 60-60, 62-62, 63-63, 64-64, 65-65
Signed-off-by: Plakhotnikov Vladimir <v.plahotnikov@yclients.tech>
Signed-off-by: Plakhotnikov Vladimir <v.plahotnikov@yclients.tech>
Signed-off-by: Plakhotnikov Vladimir <v.plahotnikov@yclients.tech>
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- .github/ISSUE_TEMPLATE/1_Bug_report.md (1 hunks)
- .github/PULL_REQUEST_TEMPLATE.md (1 hunks)
- src/Environment.php (3 hunks)
- src/Worker.php (1 hunks)
Additional context used
LanguageTool
.github/ISSUE_TEMPLATE/1_Bug_report.md
[style] ~19-~19: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 405 characters long)
Context: ...-----------| | Package Version | x.y.z | | PHP version | x.y.z | | Operating system | Linux | ....github/PULL_REQUEST_TEMPLATE.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ... | | New feature? | ✔️/❌ <!-- please update "Other Features" section in CHANGELOG.md ...
[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ... | | Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if...
PHPStan
src/Environment.php
19-19: Cannot import type alias ModeType: class Spiral\RoadRunner\Environment\Mode does not exist.
19-19: Class Spiral\RoadRunner\Environment implements unknown interface Spiral\RoadRunner\EnvironmentInterface.
src/Worker.php
32-32: Class Spiral\RoadRunner\Worker implements unknown interface Spiral\RoadRunner\StreamWorkerInterface.
46-46: Parameter $relay of method Spiral\RoadRunner\Worker::__construct() has invalid type Spiral\Goridge\RelayInterface.
46-46: Property Spiral\RoadRunner\Worker::$relay has unknown class Spiral\Goridge\RelayInterface as its type.
48-48: Instantiated class Spiral\RoadRunner\Logger not found.
48-48: Parameter $logger of method Spiral\RoadRunner\Worker::__construct() has invalid type Psr\Log\LoggerInterface.
48-48: Property Spiral\RoadRunner\Worker::$logger has unknown class Psr\Log\LoggerInterface as its type.
51-51: Call to static method register() on an unknown class Spiral\RoadRunner\Internal\StdoutHandler.
55-55: Method Spiral\RoadRunner\Worker::getLogger() has invalid return type Psr\Log\LoggerInterface.
60-60: Method Spiral\RoadRunner\Worker::waitPayload() has invalid return type Spiral\RoadRunner\Payload.
67-67: Call to static method fromFrame() on an unknown class Spiral\RoadRunner\PayloadFactory.
71-71: Class Spiral\RoadRunner\Payload not found.
73-73: Class Spiral\RoadRunner\Message\Command\WorkerStop not found.
76-76: Class Spiral\RoadRunner\Message\Command\GetProcessId not found.
79-79: Class Spiral\RoadRunner\Message\Command\Pong not found.
82-82: Class Spiral\RoadRunner\Message\SkipMessage not found.
103-103: Parameter $payload of method Spiral\RoadRunner\Worker::respond() has invalid type Spiral\RoadRunner\Payload.
111-111: Access to constant ERROR on an unknown class Spiral\Goridge\Frame.
111-111: Instantiated class Spiral\Goridge\Frame not found.
126-126: Method Spiral\RoadRunner\Worker::getPayload() has invalid return type Spiral\RoadRunner\Payload.
164-164: Class Spiral\RoadRunner\Message\Command\Pong not found.
180-180: Method Spiral\RoadRunner\Worker::pullPayload() has invalid return type Spiral\RoadRunner\Payload.
182-182: Class Spiral\Goridge\BlockingRelayInterface not found.
196-196: Call to static method fromFrame() on an unknown class Spiral\RoadRunner\PayloadFactory.
198-198: Class Spiral\RoadRunner\Message\Command\Pong not found.
208-208: Instantiated class Spiral\Goridge\Frame not found.
211-211: Access to constant BYTE10_STREAM on an unknown class Spiral\Goridge\Frame.
215-215: Access to constant BYTE10_PING on an unknown class Spiral\Goridge\Frame.
225-225: Parameter $frame of method Spiral\RoadRunner\Worker::sendFrame() has invalid type Spiral\Goridge\Frame.
228-228: Access to constant BYTE10_STREAM on an unknown class Spiral\Goridge\Frame.
229-229: Access to constant BYTE10_PING on an unknown class Spiral\Goridge\Frame.
235-235: Caught class Spiral\Goridge\Exception\GoridgeException not found.
236-236: Instantiated class Spiral\Goridge\Exception\TransportException not found.
238-238: Instantiated class Spiral\RoadRunner\Exception\RoadRunnerException not found.
251-251: Instantiated class Spiral\RoadRunner\Logger not found.
251-251: Parameter $logger of method Spiral\RoadRunner\Worker::create() has invalid type Psr\Log\LoggerInterface.
254-254: Call to static method fromGlobals() on an unknown class Spiral\RoadRunner\Environment.
265-265: Parameter $env of method Spiral\RoadRunner\Worker::createFromEnvironment() has invalid type Spiral\RoadRunner\EnvironmentInterface.
267-267: Instantiated class Spiral\RoadRunner\Logger not found.
267-267: Parameter $logger of method Spiral\RoadRunner\Worker::createFromEnvironment() has invalid type Psr\Log\LoggerInterface.
273-273: Call to static method create() on an unknown class Spiral\Goridge\Relay.
281-281: Access to constant CONTROL on an unknown class Spiral\Goridge\Frame.
281-281: Instantiated class Spiral\Goridge\Frame not found.
Additional comments not posted (3)
.github/ISSUE_TEMPLATE/1_Bug_report.md (1)
17-21
: Formatting adjustments in the table improve clarity and usability.src/Environment.php (1)
12-15
: The addition of theRR_VERSION
environment variable and thegetVersion()
method aligns with the PR objectives.Also applies to: 44-48
src/Worker.php (1)
279-281
: The change fromstatic
tovoid
in thesendProcessId
method simplifies the method's behavior.Verification successful
The necessary classes and interfaces flagged by the static analysis tool are present in the codebase. The change from
static
tovoid
in thesendProcessId
method does not introduce any unresolved dependencies.
GetProcessId
WorkerStop
StreamStop
Pong
StreamWorkerInterface
WorkerInterface
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the classes and interfaces flagged by the static analysis tool. # Test: Search for the missing classes and interfaces. Expect: Definitions found. fd --type file --exec rg --type php 'class|interface' {}Length of output: 3237
| Q | A | | ||
|--------------|-------------------------------------------------------------------------------------------------------------------------| | ||
| Bugfix? | ✔️/❌ | | ||
| Breaks BC? | ✔️/❌ <!-- please update "xxx Impact Changes" section in CHANGELOG.md file --> | | ||
| New feature? | ✔️/❌ <!-- please update "Other Features" section in CHANGELOG.md file --> | | ||
| Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead --> | | ||
| Docs PR | spiral/docs#... <!-- prefix each issue number with "spiral/docs#", required only for new features --> | |
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.
Consider adding the missing articles to enhance grammatical correctness.
- | New feature? | ✔️/❌ <!-- please update "Other Features" section in CHANGELOG.md file -->
+ | New feature? | ✔️/❌ <!-- please update the "Other Features" section in CHANGELOG.md file -->
- | Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead -->
+ | Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead -->
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| Q | A | | |
|--------------|-------------------------------------------------------------------------------------------------------------------------| | |
| Bugfix? | ✔️/❌ | | |
| Breaks BC? | ✔️/❌ <!-- please update "xxx Impact Changes" section in CHANGELOG.md file --> | | |
| New feature? | ✔️/❌ <!-- please update "Other Features" section in CHANGELOG.md file --> | | |
| Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead --> | | |
| Docs PR | spiral/docs#... <!-- prefix each issue number with "spiral/docs#", required only for new features --> | | |
| Q | A | | |
|--------------|-------------------------------------------------------------------------------------------------------------------------| | |
| Bugfix? | ✔️/❌ | | |
| Breaks BC? | ✔️/❌ <!-- please update "xxx Impact Changes" section in CHANGELOG.md file --> | | |
| New feature? | ✔️/❌ <!-- please update the "Other Features" section in CHANGELOG.md file --> | | |
| Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead --> | | |
| Docs PR | spiral/docs#... <!-- prefix each issue number with "spiral/docs#", required only for new features --> | |
Tools
LanguageTool
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ... | | New feature? | ✔️/❌ <!-- please update "Other Features" section in CHANGELOG.md ...
[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ... | | Issues | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if...
@msmakouz Could you please review? |
src/EnvironmentInterface.php
Outdated
/** | ||
* Roadrunner version | ||
*/ | ||
public function getVersion(): string; |
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.
@Kaspiman Adding a new method to the interface can break the user implementation of this interface. I suggest not adding the method to the interface, but only to the implementation, and indicating it in the interface as a comment:
/**
* Provides base values to configure roadrunner worker.
*
* @psalm-import-type ModeType from Mode
* @see Mode
* @method string getVersion()
*/
The method can then be added in the next major release of the package.
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.
Done.
Signed-off-by: Plakhotnikov Vladimir <v.plahotnikov@yclients.tech>
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.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
src/EnvironmentInterface.php (1)
Line range hint
24-24
: TheExpectedValues
attribute is reported as non-existent by the static analysis tool. This could indicate a missing import or an issue with the project's dependencies.Please ensure that the
JetBrains\PhpStorm
namespace is correctly referenced and that any necessary dependencies are included in your project.Tools
PHPStan
17-17: Cannot import type alias ModeType: class Spiral\RoadRunner\Environment\Mode does not exist.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/EnvironmentInterface.php (1 hunks)
Additional context used
PHPStan
src/EnvironmentInterface.php
17-17: Cannot import type alias ModeType: class Spiral\RoadRunner\Environment\Mode does not exist.
24-24: Attribute class JetBrains\PhpStorm\ExpectedValues does not exist.
24-24: Class Spiral\RoadRunner\Environment\Mode not found.
Additional comments not posted (2)
src/EnvironmentInterface.php (2)
15-15
: The use of@method string getVersion()
in the PHPDoc is a good interim solution to avoid breaking changes while still documenting the existence of the new method. This approach maintains backward compatibility until the next major release.
Line range hint
24-24
: As previously mentioned, theMode
class is not found. This affects theExpectedValues
attribute's functionality.Please refer to the earlier comment regarding the
Mode
class.Tools
PHPStan
17-17: Cannot import type alias ModeType: class Spiral\RoadRunner\Environment\Mode does not exist.
@@ -12,6 +12,7 @@ | |||
* | |||
* @psalm-import-type ModeType from Mode | |||
* @see Mode | |||
* @method string getVersion() | |||
*/ | |||
interface EnvironmentInterface |
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.
The static analysis tool reports that the Mode
class does not exist. This could be a genuine issue if the class has been moved or renamed, or it could be a configuration error in the analysis tool.
Please verify the existence and correct namespace of the Mode
class. If it has been moved or renamed, update the import statement accordingly.
Tools
PHPStan
17-17: Cannot import type alias ModeType: class Spiral\RoadRunner\Environment\Mode does not exist.
@Kaspiman Thanks. |
Roadrunner has
RR_VERSION
env variable, so let's add it to Environment class like otherRR_*
variables.Summary by CodeRabbit
New Features
Bug Fixes
Tests