refactor02 #10
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "oddlid/sensebot-go:refactor02"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Linter fixes, TODOs and other refactoring.
7ef516601d33f8763fbc@ -0,0 +15,4 @@// example in the errgroup docs struck me as bound to deadlock, due to unbuffered// channels and calling Wait before consuming the channel. And I was right.func Test_MenuPopulateWithErrGroupConcept(t *testing.T) {t.Skip("Manual test, disabled")If this test is just unconditionally skipped it should probably just not be included
Yes, there's a lot I'm intending to remove before merge, like this one. Just keeping it temporarily as notes to self, so I don't forget what I was thinking.
I tend to write tests just to figure out concepts, and then delete them when I've either implemented it properly, or just moved on to another solution.
But good that you remind me! There will for sure be more like this, so just keep commenting :)
@ -67,0 +68,4 @@// the button order gets jumbled after the first paging operation.// After a paging operation in EX, one can observe the buttons being out of order directly after a// click, but they sort and rearrange themselves back in order quickly.// I have no clue as to why this is.I'm pretty sure gomuks sorts them lexicographically, element tries to respect the order of addition but doesn't do very well with how much the timeline shifts
In the future, something like Bot buttons & conversations will provide a better interface
WIP: refactor02to refactor02Looks good, thanks for squashing those TODOs. Just a few nits
@ -68,6 +70,41 @@ func (bot *Client) CloseDB() error {return bot.db.Close()}func (bot *Client) Go(f func()) {nit: I know this is basically an alias for
bot.wg.Go, but naming the functionGoroutineorToGoroutineetc might be more intuitive or otherwise semantically clear, such as:@ -0,0 +41,4 @@}}// NewGeneratorFromIterator is just like NewGeneratorFromList, exceppt it takes an iteratorexceppt->except@ -0,0 +107,4 @@}// NewWaitGroupWorker is nearly identical to NewDefaultWorker, except it uses the passed// WaitGroup to spawn goroutines, so that the caller may call wg.Wait() in the end.Personally, I'd just make
NewDefaultWorkertake anilablesync.WaitGroup, creating its own if the passed one isnil. Alternatively, makingNewDefaultWorkercallNewWaitGroupWorkerwith its own WaitGroup, to improve code re-use surface, and reduce the amount of duplicated logicThank you!