EEBus: add controlbox test#28908
Merged
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
replace github.com/enbility/eebus-go => ../eebus-goentry in go.mod hardcodes a local path and will break consumers/CI that don’t have that directory layout; consider removing it or gating it behind a local-only config. - The enbility dependencies are now pinned to pseudo-versions with future timestamps; double-check that these are intentional release candidates rather than local or temporary builds before merging.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `replace github.com/enbility/eebus-go => ../eebus-go` entry in go.mod hardcodes a local path and will break consumers/CI that don’t have that directory layout; consider removing it or gating it behind a local-only config.
- The enbility dependencies are now pinned to pseudo-versions with future timestamps; double-check that these are intentional release candidates rather than local or temporary builds before merging.
## Individual Comments
### Comment 1
<location path="go.mod" line_range="270" />
<code_context>
replace go.yaml.in/yaml/v4 => go.yaml.in/yaml/v4 v4.0.0-rc.3
+
+replace github.com/enbility/eebus-go => ../eebus-go
</code_context>
<issue_to_address>
**issue (bug_risk):** Local replace directive will break consumers who don’t have ../eebus-go.
This hardcodes your local directory structure into go.mod and will fail for others or in CI where `../eebus-go` isn’t present. If it’s only needed for local development, keep it out of the committed go.mod (e.g., via a local GOWORK/GOFLAGS setup or another local-only config).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| replace go.yaml.in/yaml/v4 => go.yaml.in/yaml/v4 v4.0.0-rc.3 | ||
|
|
||
| replace github.com/enbility/eebus-go => ../eebus-go |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Local replace directive will break consumers who don’t have ../eebus-go.
This hardcodes your local directory structure into go.mod and will fail for others or in CI where ../eebus-go isn’t present. If it’s only needed for local development, keep it out of the committed go.mod (e.g., via a local GOWORK/GOFLAGS setup or another local-only config).
Member
Author
|
@copilot resolve the merge conflicts in this pull request |
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
replace github.com/enbility/eebus-go => ../eebus-goingo.modhardcodes a local filesystem path, which will break builds for other environments/CI; this should be removed or guarded (e.g., via a separate local-onlygo.modor build tooling) before merging. - The updated enbility dependencies are pinned to pseudo-versions (with future timestamps) rather than stable tags; consider updating these to the final released versions before merging to avoid depending on transient commits.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `replace github.com/enbility/eebus-go => ../eebus-go` in `go.mod` hardcodes a local filesystem path, which will break builds for other environments/CI; this should be removed or guarded (e.g., via a separate local-only `go.mod` or build tooling) before merging.
- The updated enbility dependencies are pinned to pseudo-versions (with future timestamps) rather than stable tags; consider updating these to the final released versions before merging to avoid depending on transient commits.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
# Conflicts: # go.mod # go.sum # server/eebus/eebus.go # server/eebus/test/controlbox.go
Contributor
Conflicts are resolved by merging the latest |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a controlbox unit test. Depends on enbility release.