Skip to content

Add basic changelog API support#68

Open
Chiffario wants to merge 1 commit into
MaxOhn:mainfrom
Chiffario:main
Open

Add basic changelog API support#68
Chiffario wants to merge 1 commit into
MaxOhn:mainfrom
Chiffario:main

Conversation

@Chiffario
Copy link
Copy Markdown

Adds support for /changelog and /changelog/{stream}/{build} with baseline coverage. Not entirely sure on cleanliness, but this should be good enough as a base

Handy for user count data by the way

Adds support for /changelog and /changelog/{stream}/{build} with
baseline coverage
Copy link
Copy Markdown
Owner

@MaxOhn MaxOhn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got some minor nits and clippy seems to be angery currently. Other than that, looks good

Comment thread src/model/changelog.rs
Comment on lines +8 to +9
#[derive(Clone, Debug, Deserialize)]
pub struct ChangelogListing {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add the attribute #[cfg_attr(feature = "serialize", derive(serde::Serialize))] to this and other types

Comment thread src/model/mod.rs
/// User related types
pub mod user;

pub mod changelog;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module is public, a simple comment for documentation would be nice even if it's trivial

Comment thread src/request/changelog.rs
Comment on lines +56 to +57
// There are only two supported formats, it should be fine
message_formats: Vec::with_capacity(2).into(),
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We never push into the Vec, its setter overwrites it. Vec::new should be sufficient here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants