feat: init policy defaults to true #459
Conversation
There was a problem hiding this comment.
Good first step.
However, I believe this won't work for ports that are exported on a composition (since they are not using this codepath). I don't think it would work in many cases related to compositions (such as e.g. a composition that has a service as child and gets its real component as an injection in profiles).
The more generic place to resolve this would be where we compute the final connection policy. https://github.com//rock-core/tools-syskit/blob/b674ba903538d229ded38c5980cfd5549535e4eb/lib/syskit/network_generation/dataflow_dynamics.rb#L525
And it will definitely not work for data readers. For data readers, you would need to work here: https://github.com//rock-core/tools-syskit/blob/6b0d92aa0c8bc62a79ce6d58dffb330abdfcceb9/lib/syskit/dynamic_port_binding.rb#L205
lib/syskit/models/port.rb
Outdated
| raise WrongPortConnectionTypes.new(self, in_port), "cannot connect #{out_port} to #{in_port}: types mismatch" | ||
| end | ||
|
|
||
| validate_connection!(out_port, in_port) |
There was a problem hiding this comment.
This is subjective, but while reading about ! methods in ruby, I learned that this is a convention in rails for methods that might raise exceptions (example: https://api.rubyonrails.org/classes/ActiveRecord/Persistence.html#method-i-save-21). Seemed like a practical way to indicate that the method might interrupt the normal flow of code.
But now that you mention it, maybe it doesn't make sense here, since it's not being called by the user. It does draw attention to the method, though, and that was the main idea.
There was a problem hiding this comment.
It's a ... long discussion in the Ruby circles. The ! is generally meant to represent "unexpectedly dangerous methods".... Not a very easy criteria to apply. For instance, in containers, it's used to mark self-modifying versions of modification methods (e.g. Hash#merge! vs. Hash#merge)
Regardless of the overall discussion, it's best to always follow the conventions of the codebase you're modifying. Within Syskit, the validate_* methods all raise and none has a !.
eduardacoppo
force-pushed
the
init_policy_defaults_to_true
branch
from
78b2475 to
4132b18
Compare
fix Metrics/CyclomaticComplexity offense by moving the connection validation code to a separate function
and rewrite code in port model
Use init_policy from the port as a default for the policy Do not override the existing init flag in the policy `policy.fetch(:init, port.model.init_policy)` to set the init value if not already provided in the policy
eduardacoppo
force-pushed
the
init_policy_defaults_to_true
branch
from
4132b18 to
7ab7ee8
Compare
eduardacoppo
force-pushed
the
init_policy_defaults_to_true
branch
from
a54ec92 to
945bf58
Compare
The
:initpolicy flag requests that the connection sends the last value written to it.This PR applies the
:initpolicy using therecommend_initflagDepends on: