Conversation
7b8b6d4 to
bf27dcb
Compare
bf27dcb to
779d63d
Compare
There was a problem hiding this comment.
Pull request overview
This pull request removes support for openHAB versions prior to 5.0, streamlining the codebase by eliminating version-specific compatibility code and updating dependencies.
Key Changes:
- Updated minimum openHAB version requirement from 4.1.0 to 5.0.0
- Updated Ruby version requirement from 3.1.4 to 3.4.2
- Removed version guards and conditional logic for OH < 5.0 throughout the codebase
Reviewed changes
Copilot reviewed 43 out of 44 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| openhab-scripting.gemspec | Updated Ruby version to 3.4.2 and openHAB dependency to >= 5.0.0 |
| lib/openhab/core.rb | Updated version check to require openHAB >= 5.0.0 |
| lib/openhab/core/**/*.rb | Removed OH < 5.0 compatibility code paths and version checks |
| lib/openhab/dsl/**/*.rb | Simplified method signatures using * and ** forwarding, removed deprecated code |
| lib/openhab/rspec/**/*.rb | Removed require "set" statements and version-specific test helpers |
| spec/**/*_spec.rb | Removed version guards from tests, simplified test conditions |
| features/assets/Holidays_gb_4.1.xml | Removed OH 4.1-specific holiday configuration file |
| .rubocop.yml | Updated target Ruby version to 3.4 |
| .github/workflows/*.yml | Updated Ruby version to 3.4.2, removed OH < 5.0 from CI matrix |
| Gemfile.lock | Updated dependencies and platforms for Ruby 3.4.2/Bundler 4.0.3 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/openhab/dsl/sitemaps/builder.rb
Outdated
| widget.send_frequency = (frequency * 1000).to_i if frequency | ||
| # @deprecated OH 4.1 remove the version check when dropping OH 4.1 support | ||
| widget.release_only = release_only? if OpenHAB::Core.version >= OpenHAB::Core::V4_2 && !@release_only.nil? | ||
| widget.release_only = release_only? |
There was a problem hiding this comment.
The release_only property is now being set unconditionally without checking if it's nil. This differs from the previous behavior which only set it when not nil. If @release_only is nil, release_only? will return nil, and widget.release_only = nil will be executed. This should be verified to ensure nil is a valid value for this property or the nil check should be restored.
| widget.release_only = release_only? | |
| widget.release_only = release_only? unless @release_only.nil? |
779d63d to
fd9d4ab
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 44 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def initialize(*, type: nil, function: nil, thing: nil, **) | ||
| raise ArgumentError, "invalid function #{function}" if function && !function.match?(FUNCTION_REGEX) | ||
|
|
||
| super(type, *args, **kwargs) | ||
| super(type, *, **) |
There was a problem hiding this comment.
The parameter forwarding pattern is problematic. The method signature initialize(*, type: nil, function: nil, thing: nil, **) captures all positional arguments in * and remaining keyword arguments in **. However, super(type, *, **) attempts to pass the keyword argument type (which defaults to nil) as the first positional parameter, then forward the anonymous positional arguments captured by *. This will result in type being nil when passed to the parent constructor, while the actual positional arguments (like name and label) are forwarded after it, causing a mismatch in the expected parameter order. The parent ItemBuilder#initialize expects (type, name, label, ...) with type as the first positional argument, not a keyword argument.
fd9d4ab to
abb63fa
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 44 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
abb63fa to
0f417f2
Compare
Signed-off-by: Cody Cutrer <cody@cutrer.us>
0f417f2 to
2d1e927
Compare
No description provided.