Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/SerialTest/+/39083 )
Change subject: Add rough approximation of SerialTest scaffold ......................................................................
Patch Set 5:
(5 comments)
https://review.coreboot.org/c/SerialTest/+/39083/5/drivers/servo/servo.go File drivers/servo/servo.go:
https://review.coreboot.org/c/SerialTest/+/39083/5/drivers/servo/servo.go@16 PS5, Line 16: // This package is a singleton, so global variables are no problem.
could we ever want to have the ability to run more than one server from one program? it seems you ar […]
It's supposed to be a program that runs a bunch of tests and returns a log. We have enough persistent processes in testlab scheduling, we don't need another one.
https://review.coreboot.org/c/SerialTest/+/39083/5/drivers/servo/servo.go@29 PS5, Line 29: /*
I'd consider making a servo driver which embeds the other Drivers. […]
In the end we still need some abstraction for individual device handlers because not everything using this framework will be using a servo. Servo is just the first implementation.
https://review.coreboot.org/c/SerialTest/+/39083/5/drivers/servo/servo.go@64 PS5, Line 64: func ConsoleDriver() *drivers.ConsoleDriver {
I would consider separating out the creation from the IO. […]
For a persistent process, sure. But what good is a non-started driver in a one-shot process? Ideally it wouldn't be instantiated in the first place.
https://review.coreboot.org/c/SerialTest/+/39083/5/eve.toml File eve.toml:
https://review.coreboot.org/c/SerialTest/+/39083/5/eve.toml@2 PS5, Line 2: ConsoleDriverName = "servo"
same thing here: you're in a package called driver(s), and the repetition is frowned upon. […]
Done
https://review.coreboot.org/c/SerialTest/+/39083/5/serialtest.go File serialtest.go:
https://review.coreboot.org/c/SerialTest/+/39083/5/serialtest.go@34 PS5, Line 34: ConsoleDriverName string
I still feel like the Driver word should not be here: […]
I changed the naming, but still puzzling over the "concrete types became Drivers" part.