Skip to content

Commit

Permalink
chore: added sink review checklist (vectordotdev#17799)
Browse files Browse the repository at this point in the history
This adds a checklist to our docs folder that can be useful whilst
reviewing code for sinks.

I'm sure this is not yet a definitive list, and any suggestions would be
very welcome!

---------

Signed-off-by: Stephen Wakely <[email protected]>
  • Loading branch information
StephenWakely authored Jul 11, 2023
1 parent 7774c49 commit 7f45949
Showing 1 changed file with 38 additions and 0 deletions.
38 changes: 38 additions & 0 deletions docs/REVIEWING.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,42 @@ following items should also be checked:
- [ ] Does it comply with [component spec](specs/component.md)?
- [ ] Does it comply with the [instrumentation spec](specs/instrumentation.md)?


### Checklist - new sink

This checklist is specific for Vector's sink code.

#### Logic

- [ ] Does it work? Do you understand what it is supposed to be doing?
- [ ] Does the retry logic make sense?
- [ ] Are the tests testing that the sink is emitting the correct metrics?
- [ ] Are there integration tests?

#### Code structure

- [ ] Is it using the sink prelude (`use crate::sinks::prelude::*`)?
- [ ] Is the sink a stream based sink?
Check that the return value from `SinkConfig::build` is the return from `VectorSink::from_event_streamsink`.
- [ ] Is it gated by sensible feature flags?
- [ ] Is the code modularized into `mod.rs`, `config.rs`, `sink.rs`, `request_builder.rs`, `service.rs`
- [ ] Does the code follow our [style guidelines].

#### Documentation

- [ ] Look at the doc preview on Netlify. Does it look good?
- [ ] Is there a `cue` file linking to `base`?
- [ ] Is there a markdown file under `/website/content/en/docs/reference/configuration/sinks/`?
- [ ] Are module comments included in `mod.rs` linking to any relevant areas in the external services documentation?

#### Configuration

- [ ] Are TLS settings configurable?
- [ ] Are the Request settings configurable?
- [ ] Should it have proxy settings? If so, are they in place?
- [ ] Does it need batch settings? If so, are they used?


## Expectations

We endeavour to review all PRs within 2 working days (Monday to Friday) of submission.
Expand Down Expand Up @@ -131,3 +167,5 @@ your best judgment, some code requires more testing than others depending
on its importance.

For integrations, consider whether the code could be integration tested.

[style guidelines]: https://github.com/vectordotdev/vector/blob/master/STYLE.md

0 comments on commit 7f45949

Please sign in to comment.