-
-
Notifications
You must be signed in to change notification settings - Fork 10
V2.0.5 #141
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
Conversation
fix: Update __invoke method to use UNDEFINED_STATE_VARIABLE constant …
Updated versioning and installation instructions in CHANGELOG.md.
Updated documentation link for component meta reference.
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 releases version 2.0.5 of the PhpSPA framework, marking it as the current stable version. The release includes improvements to state management, HTTP client functionality, and updates deployment/documentation URLs.
- Improved state management by replacing sentinel value checking with
func_num_args()for more robust argument detection - Added Unix socket support for HTTP client requests
- Changed SSL verification default from disabled to enabled for enhanced security
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/references/index.md | Updated deployment URL from Vercel to Railway App |
| badge/loc.svg | Updated lines of code count from 16,124 to 16,127 |
| app/core/Helper/StateManager.php | Refactored __invoke() to use func_num_args() instead of sentinel value for detecting missing arguments |
| app/core/Client/PendingRequest.php | Added unixSocketPath() method for Unix domain socket configuration |
| app/core/Client/CurlHttpClient.php | Changed SSL verification default to true and applied minor formatting improvements |
| CHANGELOG.md | Updated version status, added installation instructions, and changed documentation URLs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public function unixSocketPath(string $path): PendingRequest | ||
| { | ||
| $this->options['unix_socket_path'] = $path; | ||
| return $this; |
Copilot
AI
Jan 2, 2026
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.
This method duplicates the functionality of the unixSocket() method at lines 140-144. Both methods set the same option unix_socket_path and have identical behavior. The unixSocket() method has comprehensive documentation while this method has none. Consider removing this duplicate method to avoid confusion and maintain a single, well-documented API.
| public function unixSocketPath(string $path): PendingRequest | |
| { | |
| $this->options['unix_socket_path'] = $path; | |
| return $this; | |
| /** | |
| * Alias for {@see PendingRequest::unixSocket()}. | |
| * | |
| * @deprecated Use {@see PendingRequest::unixSocket()} instead. | |
| * | |
| * @param string $path Absolute or relative filesystem path to the Unix socket file. | |
| * @return PendingRequest Returns the current instance for fluent chaining. | |
| */ | |
| public function unixSocketPath(string $path): PendingRequest | |
| { | |
| return $this->unixSocket($path); |
| * @param mixed $value Optional value to be processed when the object is invoked. | ||
| * @return mixed The result of the invocation, depending on the implementation. | ||
| */ |
Copilot
AI
Jan 2, 2026
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 docblock is now outdated. It mentions an optional $value parameter that no longer exists in the method signature. The docblock should be updated to reflect that the method now uses variadic argument checking with func_num_args() and func_get_arg() instead of a named parameter with a default value.
| curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, $options['verify_ssl'] ?? true); | ||
| curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, ($options['verify_ssl'] ?? true) ? 2 : 0); |
Copilot
AI
Jan 2, 2026
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.
Changing the default SSL verification from false to true is a breaking change for existing code that relies on the current default behavior. This could cause existing HTTP requests to fail if they were connecting to endpoints with self-signed certificates or other SSL issues. While enabling SSL verification by default is a security best practice, this change should be clearly documented as a breaking change in the changelog and potentially versioned as a major release (v3.0.0) rather than a patch release (v2.0.5).
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.
@copilot open a new pull request to apply changes based on this feedback
|
@copilot open a new pull request to apply changes based on the comments in this thread |
No description provided.