Conversation
This commit introduces codebase.md, a comprehensive developer guide tailored for both human contributors and AI agents. It details the bot's driver-based architecture, the database abstraction layer (and the 'long slog' migration from MongoDB to BoltDB), and provides guidance on creating new features and testing handlers. The documentation maintains a 'funny but accurate' tone as requested, incorporating project history and the collective disdain for MongoDB. Co-authored-by: candour <4670475+candour@users.noreply.github.com>
…779322245 Add codebase.md developer documentation
- Added !stalk, !stalkoff, and !stalkkey commands. - Implemented background poller to check flight status every 10 minutes. - Integrated with AviationStack API for real-time status updates. - Supports multi-network bot instances by tracking flights per server connection. - Automatically stops tracking after landing or 24 hours. Co-authored-by: candour <4670475+candour@users.noreply.github.com>
|
|
||
| key := conf.Ns(flightsNs).String(apiKeyKey) | ||
| if key == "" { | ||
| logging.Error("AviationStack API key not set. Use !stalkkey to set it.") |
There was a problem hiding this comment.
sp0rkle doesn't use !-prefixed commands, so this should read sp0rkle: stalkkey <key>.
| tracking map[string]*flightInfo | ||
| } | ||
|
|
||
| func (fp *flightPoller) Poll(ctxs []*bot.Context) { |
There was a problem hiding this comment.
This function is substantial and should be broken up into a bunch of additional private methods on the flightPoller struct, A good place to start would be any blocks of code that need to hold the flight poller's internal lock, then you can defer the unlock.
| cache[info.FlightNum] = res | ||
| } | ||
|
|
||
| if res.err != nil { |
There was a problem hiding this comment.
This will log the same error multiple times if the flight number is in toProcess multiple times.
Given the other comment on R93, perhaps it would be better if *flightPoller contained effectively a map[string][]*flightInfo where the top-level map key is just the flight number. We wouldn't need the cache at all because we would only fetch each flight's information once. *flightInfo would still contain Target and the Me field should be renamed Network and be populated with ctx.Network(). It should then be trivial to loop over all of the infos for each flight and update each target once the flight's info has been fetched.
| if res.status != info.LastState { | ||
| // Find the correct context for this flight | ||
| var targetCtx *bot.Context | ||
| for _, ctx := range ctxs { |
There was a problem hiding this comment.
I was originally going to suggest transforming the input ctxs []*bot.Context to a map[string]*bot.Context, but I am pretty sure you cannot make the assumption the bot will use different nicknames on different servers. Quite the opposite, in fact -- it is way more likely to be called sp0rkle everywhere. So this mechanism for identifying which server you're talking to is going to be problematic.
To solve this properly I think we need to have some idea of the "logical network" associated with each client connection. goirc itself doesn't yet support 005 protocol support message parsing, but we could augment *bot.Context to have a Network() string method that, for now, returns the value of ctx.conn.Cfg().Server.
You can use this string as the map key in the suggested map[string]*bot.Context so that when you loop over the list of *flightInfo structs for each flight it's easy to find the appropriate context for each struct.
| FlightStatus string `json:"flight_status"` | ||
| Departure struct { | ||
| Airport string `json:"airport"` | ||
| Delay interface{} `json:"delay"` |
There was a problem hiding this comment.
Please use any instead of interface{} here and elsewhere. I know the rest of the codebase is still Ancient Go but we've gotta start somewhere ;-)
| for _, info := range toProcess { | ||
| res, ok := cache[info.FlightNum] | ||
| if !ok { | ||
| status, rawStatus, err := getFlightStatus(info.FlightNum, key) |
There was a problem hiding this comment.
It might be appropriate to have some sort of delay between calls to getFlightStatus, so we don't hit their API endpoint as fast as it can return results. Sleeping a second between each call would be the polite thing to do -- the poller runs in its own goroutine so it won't block the rest of the bot.
| @@ -0,0 +1,110 @@ | |||
| # sp0rkle Developer Guide: How to Teach an Old Bot New Tricks | |||
There was a problem hiding this comment.
Isn't this also in the previous PR? Please remove it from this PR, we can handle adding the doc separately.
There was a problem hiding this comment.
Yeah I did this all from my phone. Will try and fix this when I'm at my PC at home
|
@jules please deal with all the previous comments |
3 participants
This change is