codemonitors: Clean up CodeMonitorStore
Created by: camdencheek
Welcome to this monster housecleaning PR for CodeMonitor store. I started making small housecleaning PRs to bring our CodeMonitorStore up to current standards and make it a more idiomatic store, but there were many changes, so I decided it was better to rip off the band-aid and make one massive PR with all the changes. I could make these commits all separate PRs, but then it would take all week to get them reviewed, rebased, and merged.
A brief tour of what you'll find in this PR:
- Renaming methods to be more consistent and read more clearly (see Glossary for details)
- Inlining one-off query creation methods
- Use
QueryRow
where applicable for easier scanning/error handling - Reuse defined list of columns between queries and scanners to ensure they always agree
- Make all integer types explicitly agree with the database integer types (
integer
/serial
->int32
andbigint
/bigserial
->int64
) - Always close
*sql.Rows
in the function where it was created - Add comments
- Removed named return parameters
- Remove predeclared variables where not needed
- Add some comments (probably could still use more, particularly with the info in the glossary below)
Glossary:
-
Trigger
: an event that "triggers" a set of code monitor actions. Currently our only trigger is aQueryTrigger
. -
Action
: a response to a trigger event. Currently, our only action isEmailAction
, but this will be expanded very soon to includeWebhook
andSlackWebhook
. -
Job
: an asynchronously scheduled task that is executed by a worker. In the context of code monitors, this will be either a trigger job (execute a search and see if there are results) or an action job (send an email, execute a webhook, etc.) -
TriggerJob
: an execution of a trigger check to see if there are any new events. In this case, all trigger jobs are search executions. -
ActionJob
: an execution of a configuredAction
. Currently, only "send an email". -
QueryTrigger
: aTrigger
that checks if the results of a commit or diff search returns any new results -
EmailAction
: anAction
that sends an email to the configured recipient in response to an event from aTrigger
Review notes:
- Each commit is small, self-contained, and (I think) self-explanatory, so if you'd like to review this closely, I recommend looking at it commit-by-commit.
- These changes are purely style changes. This should be no external change to behavior.
What this PR does not cover:
- Removing dependency on the
graphqlbackend
package - Moving the store into
enterprise/internal/database
(dependent on the removal ofgraphqlbackend
dependency) - Removing dependency on the
workerutil
package - Remove
dbconn.Global
in tests - Use database
now()
rather than passing it in the query. This might not be reasonable depending on tests.