From 58cc0a9662e1bd087c2910eb15aa7568f088bba5 Mon Sep 17 00:00:00 2001
From: DanConwayDev
Date: Tue, 2 Jul 2024 08:54:55 +0100
Subject: [PATCH] feat(send): tag each maintainer's repo event
instead of just tagging the first maintainer's repo event and each
maintainer with a p tag
This allows for easier discoverability of the proposal when:
* the first maintainer hasn't issued a repo event
* the maintainers change over time and the single tagged repo event
is no listed as a maintainer in anyone elses repo event
---
src/sub_commands/send.rs | 252 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------------------------------------
test_utils/src/lib.rs | 3 ++-
tests/send.rs | 16 ++++++++++++++--
3 files changed, 150 insertions(+), 121 deletions(-)
diff --git a/src/sub_commands/send.rs b/src/sub_commands/send.rs
index 7c8f2ee..410e119 100644
--- a/src/sub_commands/send.rs
+++ b/src/sub_commands/send.rs
@@ -603,15 +603,13 @@ pub async fn generate_cover_letter_and_patch_events(
commits.len()
),
[
+ repo_ref.maintainers.iter().map(|m| Tag::coordinate(Coordinate {
+ kind: nostr::Kind::Custom(REPO_REF_KIND),
+ public_key: *m,
+ identifier: repo_ref.identifier.to_string(),
+ relays: repo_ref.relays.clone(),
+ })).collect::
vec![
- // TODO: why not tag all maintainer identifiers?
- Tag::coordinate(Coordinate {
- kind: nostr::Kind::Custom(REPO_REF_KIND),
- public_key: *repo_ref.maintainers.first()
- .context("repo reference should always have at least one maintainer")?,
- identifier: repo_ref.identifier.to_string(),
- relays: repo_ref.relays.clone(),
- }),
Tag::from_standardized(TagStandard::Reference(format!("{root_commit}"))),
Tag::hashtag("cover-letter"),
Tag::custom(
@@ -885,124 +883,142 @@ pub async fn generate_patch_event(
.context("failed to get parent commit")?;
let relay_hint = repo_ref.relays.first().map(nostr::UncheckedUrl::from);
- sign_event(EventBuilder::new(
- nostr::event::Kind::Custom(PATCH_KIND),
- git_repo
- .make_patch_from_commit(commit,&series_count)
- .context(format!("cannot make patch for commit {commit}"))?,
- [
- vec![
- Tag::coordinate(Coordinate {
- kind: nostr::Kind::Custom(REPO_REF_KIND),
- public_key: *repo_ref.maintainers.first()
- .context("repo reference should always have at least one maintainer - the issuer of the repo event")
- ?,
- identifier: repo_ref.identifier.to_string(),
- relays: repo_ref.relays.clone(),
- }),
- Tag::from_standardized(TagStandard::Reference(root_commit.to_string())),
- // commit id reference is a trade-off. its now
- // unclear which one is the root commit id but it
- // enables easier location of code comments againt
- // code that makes it into the main branch, assuming
- // the commit id is correct
- Tag::from_standardized(TagStandard::Reference(commit.to_string())),
- Tag::custom(
- TagKind::Custom(std::borrow::Cow::Borrowed("alt")),
- vec![format!("git patch: {}", git_repo.get_commit_message_summary(commit).unwrap_or_default())],
- ),
- ],
-
- if let Some(thread_event_id) = thread_event_id {
- vec![Tag::from_standardized(nostr_sdk::TagStandard::Event {
- event_id: thread_event_id,
- relay_url: relay_hint.clone(),
- marker: Some(Marker::Root),
- public_key: None,
- })]
- } else if let Some(event_ref) = root_proposal_id.clone() {
- vec![
- Tag::hashtag("root"),
- Tag::hashtag("revision-root"),
- // TODO check if id is for a root proposal (perhaps its for an issue?)
- event_tag_from_nip19_or_hex(&event_ref,"proposal", Marker::Reply, false, false)?,
- ]
- } else {
+ sign_event(
+ EventBuilder::new(
+ nostr::event::Kind::Custom(PATCH_KIND),
+ git_repo
+ .make_patch_from_commit(commit, &series_count)
+ .context(format!("cannot make patch for commit {commit}"))?,
+ [
+ repo_ref
+ .maintainers
+ .iter()
+ .map(|m| {
+ Tag::coordinate(Coordinate {
+ kind: nostr::Kind::Custom(REPO_REF_KIND),
+ public_key: *m,
+ identifier: repo_ref.identifier.to_string(),
+ relays: repo_ref.relays.clone(),
+ })
+ })
+ .collect::
vec![
- Tag::hashtag("root"),
- ]
- },
- mentions.to_vec(),
- if let Some(id) = parent_patch_event_id {
- vec![Tag::from_standardized(nostr_sdk::TagStandard::Event {
- event_id: id,
- relay_url: relay_hint.clone(),
- marker: Some(Marker::Reply),
- public_key: None,
- })]
- } else {
- vec![]
- },
- // see comment on branch names in cover letter event creation
- if let Some(branch_name) = branch_name {
- if thread_event_id.is_none() {
+ Tag::from_standardized(TagStandard::Reference(root_commit.to_string())),
+ // commit id reference is a trade-off. its now
+ // unclear which one is the root commit id but it
+ // enables easier location of code comments againt
+ // code that makes it into the main branch, assuming
+ // the commit id is correct
+ Tag::from_standardized(TagStandard::Reference(commit.to_string())),
+ Tag::custom(
+ TagKind::Custom(std::borrow::Cow::Borrowed("alt")),
+ vec![format!(
+ "git patch: {}",
+ git_repo
+ .get_commit_message_summary(commit)
+ .unwrap_or_default()
+ )],
+ ),
+ ],
+ if let Some(thread_event_id) = thread_event_id {
+ vec![Tag::from_standardized(nostr_sdk::TagStandard::Event {
+ event_id: thread_event_id,
+ relay_url: relay_hint.clone(),
+ marker: Some(Marker::Root),
+ public_key: None,
+ })]
+ } else if let Some(event_ref) = root_proposal_id.clone() {
vec![
- Tag::custom(
+ Tag::hashtag("root"),
+ Tag::hashtag("revision-root"),
+ // TODO check if id is for a root proposal (perhaps its for an issue?)
+ event_tag_from_nip19_or_hex(
+ &event_ref,
+ "proposal",
+ Marker::Reply,
+ false,
+ false,
+ )?,
+ ]
+ } else {
+ vec![Tag::hashtag("root")]
+ },
+ mentions.to_vec(),
+ if let Some(id) = parent_patch_event_id {
+ vec![Tag::from_standardized(nostr_sdk::TagStandard::Event {
+ event_id: id,
+ relay_url: relay_hint.clone(),
+ marker: Some(Marker::Reply),
+ public_key: None,
+ })]
+ } else {
+ vec![]
+ },
+ // see comment on branch names in cover letter event creation
+ if let Some(branch_name) = branch_name {
+ if thread_event_id.is_none() {
+ vec![Tag::custom(
TagKind::Custom(std::borrow::Cow::Borrowed("branch-name")),
vec![branch_name.to_string()],
- )
- ]
- }
- else { vec![]}
- }
- else { vec![]},
- // whilst it is in nip34 draft to tag the maintainers
- // I'm not sure it is a good idea because if they are
- // interested in all patches then their specialised
- // client should subscribe to patches tagged with the
- // repo reference. maintainers of large repos will not
- // be interested in every patch.
- repo_ref.maintainers
+ )]
+ } else {
+ vec![]
+ }
+ } else {
+ vec![]
+ },
+ // whilst it is in nip34 draft to tag the maintainers
+ // I'm not sure it is a good idea because if they are
+ // interested in all patches then their specialised
+ // client should subscribe to patches tagged with the
+ // repo reference. maintainers of large repos will not
+ // be interested in every patch.
+ repo_ref
+ .maintainers
.iter()
.map(|pk| Tag::public_key(*pk))
.collect(),
- vec![
- // a fallback is now in place to extract this from the patch
- Tag::custom(
- TagKind::Custom(std::borrow::Cow::Borrowed("commit")),
- vec![commit.to_string()],
- ),
- // this is required as patches cannot be relied upon to include the 'base commit'
- Tag::custom(
- TagKind::Custom(std::borrow::Cow::Borrowed("parent-commit")),
- vec![commit_parent.to_string()],
- ),
- // this is required to ensure the commit id matches
- Tag::custom(
- TagKind::Custom(std::borrow::Cow::Borrowed("commit-pgp-sig")),
- vec![
- git_repo
- .extract_commit_pgp_signature(commit)
- .unwrap_or_default(),
+ vec![
+ // a fallback is now in place to extract this from the patch
+ Tag::custom(
+ TagKind::Custom(std::borrow::Cow::Borrowed("commit")),
+ vec![commit.to_string()],
+ ),
+ // this is required as patches cannot be relied upon to include the 'base
+ // commit'
+ Tag::custom(
+ TagKind::Custom(std::borrow::Cow::Borrowed("parent-commit")),
+ vec![commit_parent.to_string()],
+ ),
+ // this is required to ensure the commit id matches
+ Tag::custom(
+ TagKind::Custom(std::borrow::Cow::Borrowed("commit-pgp-sig")),
+ vec![
+ git_repo
+ .extract_commit_pgp_signature(commit)
+ .unwrap_or_default(),
],
- ),
- // removing description tag will not cause anything to break
- Tag::from_standardized(nostr_sdk::TagStandard::Description(
- git_repo.get_commit_message(commit)?.to_string()
- )),
- Tag::custom(
- TagKind::Custom(std::borrow::Cow::Borrowed("author")),
- git_repo.get_commit_author(commit)?,
- ),
- // this is required to ensure the commit id matches
- Tag::custom(
- TagKind::Custom(std::borrow::Cow::Borrowed("committer")),
- git_repo.get_commit_comitter(commit)?,
- ),
- ],
- ]
- .concat(),
- ), signer).await
+ ),
+ // removing description tag will not cause anything to break
+ Tag::from_standardized(nostr_sdk::TagStandard::Description(
+ git_repo.get_commit_message(commit)?.to_string(),
+ )),
+ Tag::custom(
+ TagKind::Custom(std::borrow::Cow::Borrowed("author")),
+ git_repo.get_commit_author(commit)?,
+ ),
+ // this is required to ensure the commit id matches
+ Tag::custom(
+ TagKind::Custom(std::borrow::Cow::Borrowed("committer")),
+ git_repo.get_commit_comitter(commit)?,
+ ),
+ ],
+ ]
+ .concat(),
+ ),
+ signer,
+ )
+ .await
.context("failed to sign event")
}
// TODO
diff --git a/test_utils/src/lib.rs b/test_utils/src/lib.rs
index ba9d30c..2825eaa 100644
--- a/test_utils/src/lib.rs
+++ b/test_utils/src/lib.rs
@@ -101,7 +101,8 @@ pub static TEST_KEY_2_NSEC: &str =
"nsec1ypglg6nj6ep0g2qmyfqcv2al502gje3jvpwye6mthmkvj93tqkesknv6qm";
pub static TEST_KEY_2_NPUB: &str =
"npub1h2yz2eh0798nh25hvypenrz995nla9dktfuk565ljf3ghnkhdljsul834e";
-
+pub static TEST_KEY_2_PUBKEY_HEX: &str =
+ "ba882566eff14f3baa976103998c452d27fe95b65a796a6a9f92628bced76fe5";
pub static TEST_KEY_2_DISPLAY_NAME: &str = "carole";
pub static TEST_KEY_2_ENCRYPTED: &str = "...2";
pub static TEST_KEY_2_KEYS: Lazy
diff --git a/tests/send.rs b/tests/send.rs
index 2c95e1e..0169c82 100644
--- a/tests/send.rs
+++ b/tests/send.rs
@@ -375,7 +375,7 @@ mod when_cover_letter_details_specified_with_range_of_head_2_sends_cover_letter_
#[tokio::test]
#[serial]
- async fn a_tag_for_repo_event() -> Result<()> {
+ async fn a_tag_for_repo_event_of_each_maintainer() -> Result<()> {
let (_, _, r53, r55, r56) = prep_run_create_proposal(true).await?;
for relay in [&r53, &r55, &r56] {
let cover_letter_event: &nostr::Event =
@@ -385,6 +385,11 @@ mod when_cover_letter_details_specified_with_range_of_head_2_sends_cover_letter_
"{REPOSITORY_KIND}:{TEST_KEY_1_PUBKEY_HEX}:{}",
generate_repo_ref_event().identifier().unwrap()
))));
+ assert!(cover_letter_event.iter_tags().any(|t| t.as_vec()[0].eq("a")
+ && t.as_vec()[1].eq(&format!(
+ "{REPOSITORY_KIND}:{TEST_KEY_2_PUBKEY_HEX}:{}",
+ generate_repo_ref_event().identifier().unwrap()
+ ))));
}
Ok(())
}
@@ -564,7 +569,7 @@ mod when_cover_letter_details_specified_with_range_of_head_2_sends_cover_letter_
#[tokio::test]
#[serial]
- async fn a_tag_for_repo_event() -> Result<()> {
+ async fn a_tag_for_repo_event_of_each_maintainer() -> Result<()> {
assert!(prep().await?.tags.iter().any(|t| {
t.as_vec()[0].eq("a")
&& t.as_vec()[1].eq(&format!(
@@ -572,6 +577,13 @@ mod when_cover_letter_details_specified_with_range_of_head_2_sends_cover_letter_
generate_repo_ref_event().identifier().unwrap()
))
}));
+ assert!(prep().await?.tags.iter().any(|t| {
+ t.as_vec()[0].eq("a")
+ && t.as_vec()[1].eq(&format!(
+ "{REPOSITORY_KIND}:{TEST_KEY_2_PUBKEY_HEX}:{}",
+ generate_repo_ref_event().identifier().unwrap()
+ ))
+ }));
Ok(())
}
--
libgit2 1.7.2