Skip to content

Conversation

@noahlevi
Copy link
Collaborator

No description provided.

@noahlevi noahlevi changed the base branch from master to launch/v1.0.0 December 23, 2025 16:33

#[cfg(feature = "backend")]
/// Parse a DOI into prefix and suffix
pub fn parse_doi(doi: &Doi) -> ThothResult<(String, String)> {
Copy link
Member

Choose a reason for hiding this comment

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

The Doi struct already performs validation, implementing FromStr (check thoth-api/src/model/mod.rs), and can output the string without the domain using to_string() (impl Display).

Overall, it'd make more sense to add a couple of methods to impl Doi in the model: fn prefix(&self) -> String and fn suffix(&self) -> String

_filter_param_3: Option<Self::FilterParameter3>,
_filter_param_4: Option<Self::FilterParameter4>,
) -> ThothResult<i32> {
Err(ThothError::InternalError(
Copy link
Member

Choose a reason for hiding this comment

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

Just use unimplemented!()

crate::model::publication::Publication::from_id(db, &publication_id)?
.publisher_id(db)
}
_ => Err(ThothError::InternalError(
Copy link
Member

Choose a reason for hiding this comment

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

I've seen a bunch of ThothError::InternalError in the PR, but these are not internal errors but user input errors... So let's just create new variants for each of them in thoth-errors

Copy link
Member

Choose a reason for hiding this comment

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

I assume this can go

Copy link
Member

Choose a reason for hiding this comment

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

you can make the whole module #[cfg(feature = "backend")] rather than each individual import/function

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.

3 participants