mab122
/
radicle-heartwood-lfs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cli: Don't create empty code review 

If the verdict and comment are both unset, return an error.
This commit is contained in:
Alexis Sellier 2024-07-06 21:05:22 +02:00
parent a32c4b93e2
commit 34112d8504
No known key found for this signature in database
2 changed files with 11 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
radicle-cob/src/object/collaboration/error.rs
View File

@ -36,7 +36,7 @@ pub struct Remove {
#[derive(Debug, Error)] #[derive(Debug, Error)]
pub enum Retrieve { pub enum Retrieve {
#[error("object failed to evaluate: {0}")] #[error(transparent)]
Evaluate(Box<dyn std::error::Error + Send + Sync + 'static>), Evaluate(Box<dyn std::error::Error + Send + Sync + 'static>),
#[error(transparent)] #[error(transparent)]
Git(#[from] git2::Error), Git(#[from] git2::Error),
@ -57,7 +57,7 @@ impl Retrieve {
#[derive(Debug, Error)] #[derive(Debug, Error)]
pub enum Update { pub enum Update {
#[error("object failed to evaluate: {0}")] #[error(transparent)]
Evaluate(Box<dyn std::error::Error + Send + Sync + 'static>), Evaluate(Box<dyn std::error::Error + Send + Sync + 'static>),
#[error("no object found")] #[error("no object found")]
NoSuchObject, NoSuchObject,

12
radicle/src/cob/patch.rs
View File

@ -103,6 +103,9 @@ pub enum Error {
/// Identity document is missing. /// Identity document is missing.
#[error("missing identity document")] #[error("missing identity document")]
MissingIdentity, MissingIdentity,
/// Review is empty.
#[error("empty review; verdict or summary not provided")]
EmptyReview,
/// Error loading the document payload. /// Error loading the document payload.
#[error("payload failed to load: {0}")] #[error("payload failed to load: {0}")]
Payload(#[from] PayloadError), Payload(#[from] PayloadError),
@ -132,7 +135,7 @@ pub enum Error {
#[source] #[source]
err: Box<dyn std::error::Error + Send + Sync + 'static>, err: Box<dyn std::error::Error + Send + Sync + 'static>,
}, },
#[error("failed to remove patch {id} from cache : {err}")] #[error("failed to remove patch {id} from cache: {err}")]
CacheRemove { CacheRemove {
id: PatchId, id: PatchId,
#[source] #[source]
@ -879,6 +882,9 @@ impl Patch {
let Some(rev) = self.revisions.get_mut(&revision) else { let Some(rev) = self.revisions.get_mut(&revision) else {
return Err(Error::Missing(revision.into_inner())); return Err(Error::Missing(revision.into_inner()));
}; };
if summary.is_none() && verdict.is_none() {
return Err(Error::EmptyReview);
}
if let Some(rev) = rev { if let Some(rev) = rev {
// Nb. Applying two reviews by the same author is not allowed and // Nb. Applying two reviews by the same author is not allowed and
// results in the review being redacted. // results in the review being redacted.
@ -2928,7 +2934,7 @@ mod test {
// It's fine to redact a review from a redacted revision. // It's fine to redact a review from a redacted revision.
let review = patch let review = patch
.review(updated, None, None, vec![], &alice.signer) .review(updated, Some(Verdict::Accept), None, vec![], &alice.signer)
.unwrap(); .unwrap();
patch.redact(updated, &alice.signer).unwrap(); patch.redact(updated, &alice.signer).unwrap();
patch.redact_review(review, &alice.signer).unwrap(); patch.redact_review(review, &alice.signer).unwrap();
@ -3160,7 +3166,7 @@ mod test {
new: Some(CodeRange::Lines { range: 5..8 }), new: Some(CodeRange::Lines { range: 5..8 }),
}; };
let review = patch let review = patch
.review(rid, None, None, vec![], &alice.signer) .review(rid, Some(Verdict::Accept), None, vec![], &alice.signer)
.unwrap(); .unwrap();
patch patch
.review_comment( .review_comment(