Conversation
|
@vaspahomov thanks for your patch. I've left a few comments in review, and it would be good to have a test for this too. |
vaspahomov
force-pushed
the
feature/add-GetUnitFileState-method
branch
2 times, most recently
from
586b8f6 to
3ef922d
Compare
vaspahomov
force-pushed
the
feature/add-GetUnitFileState-method
branch
from
3ef922d to
db69fcc
Compare
lucab
left a comment
lucab
left a comment
There was a problem hiding this comment.
Code looks fine but CI is failing. I think this is because you are trying to re-use a system service unit that may not exist on older OS version. It would be good to try with a dedicated unit instead.
| func TestGetUnitFileState(t *testing.T) { | ||
| conn := setupConn(t) | ||
| defer conn.Close() | ||
| service := "systemd-udevd.service" |
There was a problem hiding this comment.
This may not exist in the testing environment. It would be good to set up a dedicated temporary service unit instead.
|
Can you please resolve conflicts so @lucab can merge it? |
|
looking to call this method (GetUnitFileState) but it's just stale. Is there another way? |
| return status, nil | ||
| } | ||
|
|
||
| // GetUnitFileStateContext returns UnitFileState |
There was a problem hiding this comment.
- You don't have to add
Contextto the method name -- all new methods accept context. - What is
UnitFileStateand where it is defined?
|
|
||
| // GetUnitFileStateContext returns UnitFileState | ||
| func (c *Conn) GetUnitFileStateContext(ctx context.Context, name string) (string, error) { | ||
| var prop dbus.Variant |
There was a problem hiding this comment.
Can you just use string here?
|
@vaspahomov please let us know if you want to keep working on this (and I apologize for a late response). |
6 participants
No description provided.