search

scrapli / scrapligo

scrapli, but in go!
MIT License
244 stars 35 forks source link

SendCommands return early if commands len == 0 #134

Closed steiler closed 1 year ago

steiler commented 1 year ago

fixing #133

carlmontanari commented 1 year ago

👋

Thanks @steiler ! I wonder if this should actually return an error instead? Or failing that, it's probably worth making sure the default settings in the MultiResponse all make sense (as in make sense to whatever degree they can when there are no responses stored in it).

Also I didn't check yet but I assume this will be a problem in both Commands/Configs -- probably we should sort both out ya? I can do that but probably wont get to it till the weekend.

Thanks again for raising this and the pr!! 🔥

steiler commented 1 year ago

Yeah wasn't sure... An error seems odd because everything is fine it is just an empty configuration file being applied. Basically that should easily go through in my view. Not sure what it means for everything else I leave that for you to sort out. Thanks

hellt commented 1 year ago

IMHO returning error is what I'd do. Empty config payload is a no-op which makes no sense from the use case perspective, nor it bears any meaning for testing. Catching this early with returning a clear error prevent unnecessary rpcs from being made.

carlmontanari commented 1 year ago

IMHO returning error is what I'd do

I dig it.. will add a ErrNoOp or something like that error so it's very clear what happened. Will sort it this weekend if I dont get it done earlier. Thanks as always @hellt !!

carlmontanari commented 1 year ago

hey ya'll -- pushed the no-op error thing. lemme know what ya think? if everyones happy ill get that merged!

steiler commented 1 year ago

Hi Carl, I've already worked around it, now checking the length up front. So whatever you do, except for causing a panic is fine ;-)