Closed jaypipes closed 5 years ago
@chiptus @deviantony mind taking a look at this PR?
LGTM The implementation of the mergeOptions seems huge and prone to errors, but since there's a test for this I think it's ok. Would be nice to have some generic implementation you can use for ghw too. Good work, thank you for your time
On Tue, Nov 13, 2018 at 5:05 PM Jay Pipes notifications@github.com wrote:
@chiptus https://github.com/chiptus @deviantony https://github.com/deviantony mind taking a look at this PR?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jaypipes/pcidb/pull/13#issuecomment-438298263, or mute the thread https://github.com/notifications/unsubscribe-auth/ABUVF_io9v6LUalR5zaYNNWiSkL7Pn3Gks5uut-lgaJpZM4Yb1e8 .
@chiptus most of the new code is actually just moved from the discover.go module (where it was hard-coding the lookup of environ vars) to the main.go module, so not a whole lot of new code. I needed to use the pointer-to-string and pointer-to-bool stuff in order to check for the absence of values.
I was modeling the WithChroot(), WithCacheOnly() modifier functions after the way that etcd
's clientv3.KVS
class works, where you can pass zero or more WithSort()
or WithPrefix()
modifiers to methods like Op.Put()
. I found it a nice and consistent user experience that allows using the same function/method without any modifiers and still get a coherent interface.
Anyway, thanks for taking a look! :)
-jay
Ah it's merged already, sorry, TZ issue I guess :-)
Thanks @jaypipes
Adds the ability to override the CHROOT and whether to only use the cached pciids file using options passed to the
pcidb.New()
function. The chroot still defaults to the PCIDB_CHROOT environs variable value, if set, but now it's possible to set this configuration value like so:Issue #12