Account login

[kernelkind]

This PR adds pretty basic account login. It's a tad rough, but I just wanted to get it out to get feedback.

First two commits are from #62

A couple notes:

• I added nostr-sdk as a dependency to use their version of Keys, PublicKey and SecretKey. I saw that we already had a conception of PublicKey in enostr, but I didn't see a point to recreating efforts that were already built out by nostr-sdk. Let me know if anyone has thoughts about that.
• The login_state was added to Damus. When it is in state LoginState::AcquiredLogin(Keys), that represents the user's current Keys (either only public key or public & private).
• If the queries/global.json contains a valid pubkey, it will use that as the current user and set the login_state to LoginState::AcquiredLogin with that pubkey. I don't think this implementation shouldn't stay for when we implement private key authentication
• The login panel is very rough to look at. It's just a proof of concept to demonstrate MVP features for this PR

[jb55]

On Sun, Mar 24, 2024 at 02:20:12PM -0400, kernelkind wrote:

This PR adds pretty basic account login. It's a tad rough, but I just wanted to get it out to get feedback.

A couple notes:

• I added nostr-sdk as a dependency to use their version of Keys, PublicKey and SecretKey. I saw that we already had a conception of PublicKey in enostr, but I didn't see a point to recreating efforts that were already built out by nostr-sdk. Let me know if anyone has thoughts about that.

The main reason we are not using PublicKey from nostr_sdk is that it does have the same data layout, so for the majority of common operations such as comparing pubkeys in nostrdb, we first have to serialize the nostr_sdk pubkeys for each comparison.

nostr_sdk is a fairy large dependency and it doesn't really gain us much right now. Relying on some externel library that could break us any time is always a liability, especially if we are depending on their core data types which they can change at any time. I would rather not use it until we absolutely need to.

• The login_state was added to Damus. When it is in state LoginState::AcquiredLogin(Keys), that represents the user's current Keys (either only public key or public & private).

• If the queries/global.json contains a valid pubkey, it will use that as the current user and set the login_state to LoginState::AcquiredLogin with that pubkey. I don't think this implementation shouldn't stay for when we implement private key authentication

This is a bit weird for sure.

• The login panel is very rough to look at. It's just a proof of concept to demonstrate MVP features for this PR

We actually have a login design if you want to take a look at it while you work on this PR:

https://www.figma.com/file/aONNEWCxHlckZJq0lHyxrN/Notedeck?type=design&node-id=83-4153&mode=design&t=qYfYNEEFhYWdaXh6-0

Keep in mind we should be making this responsive for both mobile and desktop.

I guess one more thing:

In the current version I can pass multiple queries over the command line to build stateless UIs in notedeck. It seems like the PR breaks that.

[jb55]

This entire patch is confusing to me

On Sun, Mar 24, 2024 at 02:20:12PM -0400, kernelkind wrote:

Closes: https://github.com/damus-io/android/pull/61

.gitignore | 1 + queries/global.json | 1 - src/app.rs | 11 +++++++++-- 3 files changed, 10 insertions(+), 3 deletions(-) delete mode 100644 queries/global.json

diff --git a/.gitignore b/.gitignore index 100e65f..4eea8a9 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ src/camera.rs .txt /tags .mdb +queries/global.json

why?

diff --git a/queries/global.json b/queries/global.json deleted file mode 100644 index faf282c..0000000 --- a/queries/global.json +++ /dev/null @@ -1 +0,0 @@ -{"limit": 10, "kinds":[1]}

not sure why we're deleting this.

diff --git a/src/app.rs b/src/app.rs index 3a4542c..67f713d 100644 --- a/src/app.rs +++ b/src/app.rs @@ -12,6 +12,7 @@ use crate::widgets::note::NoteContents; use crate::Result; use egui::containers::scroll_area::ScrollBarVisibility; use std::borrow::Cow; +use std::fs;

use egui::widgets::Spinner; use egui::{ @@ -450,15 +451,21 @@ impl Damus {

    let mut timelines: Vec<Timeline> = vec![];
    let initial_limit = 100;

• let queries_json_path = "queries/global.json";

• if args.len() > 1 {
      for arg in &args[1..] {
          let filter = serde_json::from_str(&arg).unwrap();
          timelines.push(Timeline::new(filter));
      }

• } else {
• let filter = serde_json::from_str(&include_str!("../queries/global.json")).unwrap();

This is simply meant as a built-in default if there are no queries passed in the command-line. I'm not sure why we're removing this;

• } else if Path::new(queries_json_path).exists() {
• let file_content = fs::read_to_string(queries_json_path).expect("Failed to read file");
• let filter = serde_json::from_str(&file_content).expect("Failed to deserialize");

This might be important in the future, like for loading some default set of queries in the ~/.config dir, but for now you can change the default query by just passing in a query argument...

+ timelines.push(Timeline::new(filter)); //vec![get_home_filter(initial_limit)]

• } else {
• panic!("No timelines to load.");

not sure why we're introducing an unneeded panic here.

[jb55]

Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Cargo.lock b/Cargo.lock index 718a2f2..814273a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2060,7 +2060,7 @@ dependencies = [ [[package]] name = "nostrdb" version = "0.2.0" -source = "git+https://github.com/damus-io/nostrdb-rs?rev=750539d0b71ed81ec626e4670eccf34950ad2942#750539d0b71ed81ec626e4670eccf34950ad2942" +source = "git+https://github.com/damus-io/nostrdb-rs?rev=2675e7244554e40c9ee10d82b42bc647fef4c17d#2675e7244554e40c9ee10d82b42bc647fef4c17d"

thanks! not sure why this isn't updating locally for me

I've applied this for now but don't forget Signed-off-by on all your commits in the future.

[jb55]

Add login key parsing

This patch is great! lots of useful stuff in here.

On Fri, Mar 22, 2024 at 06:33:09PM -0400, kernelkind wrote:

Closes: https://github.com/damus-io/android/pull/61

src/key_parsing.rs | 235 +++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 5 + src/test_utils.rs | 36 +++++++ 3 files changed, 276 insertions(+) create mode 100644 src/key_parsing.rs create mode 100644 src/test_utils.rs

diff --git a/src/key_parsing.rs b/src/key_parsing.rs new file mode 100644 index 0000000..ee7f65c --- /dev/null +++ b/src/key_parsing.rs @@ -0,0 +1,235 @@ +use std::str::FromStr; +use std::collections::HashMap; + +use crate::Error; +use ehttp::{Request, Response}; +use nostr_sdk::{prelude::Keys, PublicKey, SecretKey}; +use poll_promise::Promise; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, PartialEq)] +pub enum LoginError {

• InvalidKey,
• Nip05Failed(String), +}

• +impl std::fmt::Display for LoginError {

• fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
• match self {
• LoginError::InvalidKey => write!(f, "The inputted key is invalid."),
• LoginError::Nip05Failed(e) => write!(f, "Failed to get pubkey from Nip05 address: {e}"),
• }
• } +}

• +impl std::error::Error for LoginError {}

• +#[derive(Deserialize, Serialize)] +pub struct Nip05Result {

• pub names: HashMap<String, String>,
• pub relays: Option<HashMap<String, Vec>>, +}

• +fn parse_nip05_response(response: Response) -> Result<Nip05Result, Error> {

• serde_json::from_slice::(&response.bytes)
• .map_err(|e| {
• Error::Generic(e.to_string())
• }) +}

• +fn get_pubkey_from_result(result: Nip05Result, user: String) -> Result<PublicKey, Error> {

• match result.names.get(&user).to_owned() {
• Some(pubkey_str) => PublicKey::from_str(pubkey_str).map_err(|e| {
• Error::Generic("Could not parse pubkey: ".to_string() + e.to_string().as_str())
• }),
• None => Err(Error::Generic("Could not find user in json.".to_string())),
• } +}

• +fn get_nip05_pubkey(id: &str) -> Promise<Result<PublicKey, Error>> {

• let (sender, promise) = Promise::new();
• let mut parts = id.split('@');
• let user = match parts.next() {
• Some(user) => user,
• None => {
• sender.send(Err(Error::Generic(
• "Address does not contain username.".to_string(),
• )));
• return promise;
• }
• };
• let host = match parts.next() {
• Some(host) => host,
• None => {
• sender.send(Err(Error::Generic(
• "Nip05 address does not contain host.".to_string(),
• )));
• return promise;
• }
• };
• if parts.next().is_some() {
• sender.send(Err(Error::Generic(
• "Nip05 address contains extraneous parts.".to_string(),
• )));
• return promise;
• }
• let url = format!("https://{host}/.well-known/nostr.json?name={user}");
• let request = Request::get(url);
• let cloned_user = user.to_string();
• ehttp::fetch(request, move |response: Result<Response, String>| {
• let result = match response {
• Ok(resp) => parse_nip05_response(resp)
• .and_then(move |result| get_pubkey_from_result(result, cloned_user)),
• Err(e) => Err(Error::Generic(e.to_string())),
• };
• sender.send(result);
• });
• promise +}

• +fn retrieving_nip05_pubkey(key: &str) -> bool {

• key.contains('@') +}

• +fn nip05_promise_wrapper(id: &str) -> Promise<Result<Keys, LoginError>> {

• let (sender, promise) = Promise::new();
• let original_promise = get_nip05_pubkey(id);
• std::thread::spawn(move || {
• let result = original_promise.block_and_take();
• let transformed_result = match result {
• Ok(public_key) => Ok(Keys::from_public_key(public_key)),
• Err(e) => Err(LoginError::Nip05Failed(e.to_string())),
• };
• sender.send(transformed_result);
• });
• promise +}

• +/// Attempts to turn a string slice key from the user into a Nostr-Sdk Keys object. +/// The key can be in any of the following formats: +/// - Public Bech32 key (prefix "npub"): "npub1xyz..." +/// - Private Bech32 key (prefix "nsec"): "nsec1xyz..." +/// - Public hex key: "02a1..." +/// - Private hex key: "5dab..." +/// - NIP-05 address: @.***" +/// +/// For NIP-05 addresses, retrieval of the public key is an asynchronous operation that returns a Promise, so it +/// will not be immediately ready. +/// All other key formats are processed synchronously even though they are still behind a Promise, they will be +/// available immediately. +/// +/// Returns a Promise that resolves to Result<Keys, LoginError>. LoginError is returned in case of invalid format, +/// unsupported key types, or network errors during NIP-05 address resolution. +/// +pub fn perform_key_retrieval(key: &str) -> Promise<Result<Keys, LoginError>> {

• let tmp_key: &str = if let Some(stripped) = key.strip_prefix('@') {
• stripped
• } else {
• key
• };
• if retrieving_nip05_pubkey(tmp_key) {
• nip05_promise_wrapper(tmp_key)
• } else {
• let result: Result<Keys, LoginError> = if let Ok(pubkey) = PublicKey::from_str(tmp_key) {
• Ok(Keys::from_public_key(pubkey))
• } else if let Ok(secret_key) = SecretKey::from_str(tmp_key) {
• Ok(Keys::new(secret_key))
• } else {
• Err(LoginError::InvalidKey)
• };
• Promise::from_ready(result)
• } +}

• +#[cfg(test)] +mod tests {

• use super::*;
• use crate::promise_assert;

• [test]

• fn test_pubkey() {
• let pubkey_str = "npub1xtscya34g58tk0z605fvr788k263gsu6cy9x0mhnm87echrgufzsevkk5s";
• let expected_pubkey = PublicKey::from_str(pubkey_str).expect("Should not have errored.");
• let login_key_result = perform_key_retrieval(pubkey_str);
• promise_assert!(
• assert_eq,
• Ok(Keys::from_public_key(expected_pubkey)),
• &login_key_result
• );
• }

• [test]

• fn test_hex_pubkey() {
• let pubkey_str = "32e1827635450ebb3c5a7d12c1f8e7b2b514439ac10a67eef3d9fd9c5c68e245";
• let expected_pubkey = PublicKey::from_str(pubkey_str).expect("Should not have errored.");
• let login_key_result = perform_key_retrieval(pubkey_str);
• promise_assert!(
• assert_eq,
• Ok(Keys::from_public_key(expected_pubkey)),
• &login_key_result
• );
• }

• [test]

• fn test_privkey() {
• let privkey_str = "nsec1g8wt3hlwjpa4827xylr3r0lccufxltyekhraexes8lqmpp2hensq5aujhs";
• let expected_privkey = SecretKey::from_str(privkey_str).expect("Should not have errored.");
• let login_key_result = perform_key_retrieval(privkey_str);
• promise_assert!(
• assert_eq,
• Ok(Keys::new(expected_privkey)),
• &login_key_result
• );
• }

• [test]

• fn test_hex_privkey() {
• let privkey_str = "41dcb8dfee907b53abc627c711bff8c7126fac99b5c7dc9b303fc1b08557cce0";
• let expected_privkey = SecretKey::from_str(privkey_str).expect("Should not have errored.");
• let login_key_result = perform_key_retrieval(privkey_str);
• promise_assert!(
• assert_eq,
• Ok(Keys::new(expected_privkey)),
• &login_key_result
• );
• }

• [test]

• fn test_nip05() {
• let nip05_str = @.***";
• let expected_pubkey =
• PublicKey::from_str("npub18m76awca3y37hkvuneavuw6pjj4525fw90necxmadrvjg0sdy6qsngq955")
• .expect("Should not have errored.");
• let login_key_result = perform_key_retrieval(nip05_str);
• promise_assert!(
• assert_eq,
• Ok(Keys::from_public_key(expected_pubkey)),
• &login_key_result
• );
• }

• [test]

• fn test_nip05_pubkey() {
• let nip05_str = @.***";
• let expected_pubkey =
• PublicKey::from_str("npub18m76awca3y37hkvuneavuw6pjj4525fw90necxmadrvjg0sdy6qsngq955")
• .expect("Should not have errored.");
• let login_key_result = get_nip05_pubkey(nip05_str);
• let res = login_key_result.block_and_take().expect("Should not error");
• assert_eq!(expected_pubkey, res);
• } +} diff --git a/src/lib.rs b/src/lib.rs index 15a1f00..05e1be6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,11 @@ mod frame_history; mod timeline; mod colors; mod profile; +mod key_parsing;

• +#[cfg(test)] +#[macro_use] +mod test_utils;

pub use app::Damus; pub use error::Error; diff --git a/src/test_utils.rs b/src/test_utils.rs new file mode 100644 index 0000000..862650d --- /dev/null +++ b/src/test_utils.rs @@ -0,0 +1,36 @@ +use poll_promise::Promise; +use std::thread; +use std::time::Duration; + +pub fn promise_wait<'a, T: Send + 'a>(promise: &'a Promise) -> &'a T {

• let mut count = 1;
• loop {
• if let Some(result) = promise.ready() {
• println!("quieried promise num times: {}", count);
• return result;
• } else {
• count += 1;
• thread::sleep(Duration::from_millis(10));
• }
• } +}

• +/// promise_assert macro +/// +/// This macro is designed to emulate the nature of immediate mode asynchronous code by repeatedly calling +/// promise.ready() for a promise, sleeping for a short period of time, and repeating until the promise is ready. +/// +/// Arguments: +/// - $assertion_closure: the assertion closure which takes two arguments: the actual result of the promise and +/// the expected value. This macro is used as an assertion closure to compare the actual and expected values. +/// - $expected: The expected value of type T that the promise's result is compared against. +/// - $asserted_promise: A Promise<T> that returns a value of type T when the promise is satisfied. This +/// represents the asynchronous operation whose result will be tested. +/// +#[macro_export] +macro_rules! promise_assert {

• ($assertion_closure:ident, $expected:expr, $asserted_promise:expr) => {
• let result = $crate::test_utils::promise_wait($asserted_promise);
• $assertion_closure!(*result, $expected);
• }; +}

[jb55]

Add login UI

This is a good start for testing, but I don't think I can merge it until we implement roberto's design.

Let's keep working on this!

On Sat, Mar 23, 2024 at 09:01:48PM -0400, kernelkind wrote:

src/app.rs | 105 ++++++++++++++++++++++++++++++++++++++++--- src/lib.rs | 1 + src/login_manager.rs | 23 ++++++++++ 3 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 src/login_manager.rs

diff --git a/src/app.rs b/src/app.rs index 67f713d..d3b4b19 100644 --- a/src/app.rs +++ b/src/app.rs @@ -5,12 +5,17 @@ use crate::fonts::{setup_fonts, NamedFontFamily}; use crate::frame_history::FrameHistory; use crate::images::fetch_img; use crate::imgcache::ImageCache; +use crate::key_parsing::perform_key_retrieval; +use crate::key_parsing::LoginError; +use crate::login_manager::LoginManager; use crate::notecache::NoteCache; use crate::timeline; use crate::ui::padding; use crate::widgets::note::NoteContents; use crate::Result; use egui::containers::scroll_area::ScrollBarVisibility; +use egui::Layout; +use nostr_sdk::PublicKey; use std::borrow::Cow; use std::fs;

@@ -21,6 +26,7 @@ use egui::{ };

use enostr::{ClientMessage, Filter, Pubkey, RelayEvent, RelayMessage}; +use nostr_sdk::Keys; use nostrdb::{ Block, BlockType, Blocks, Config, Mention, Ndb, Note, NoteKey, ProfileRecord, Subscription, Transaction, @@ -41,6 +47,11 @@ pub enum DamusState { Initialized, }

+pub enum LoginState {

• LoggingIn(LoginManager),
• AcquiredLogin(Keys), +}

• [derive(Debug, Eq, PartialEq, Copy, Clone)]

  pub struct NoteRef { pub key: NoteKey, @@ -86,6 +97,7 @@ impl Timeline { /// We derive Deserialize/Serialize so we can persist app state on shutdown. pub struct Damus { state: DamusState,

• login_state: LoginState, compose: String,

  note_cache: HashMap<NoteKey, NoteCache>, @@ -137,6 +149,13 @@ fn get_home_filter(limit: u16) -> Filter { ) }

+fn get_filter_for_pubkey(limit: u16, pubkey_hex: String) -> Filter {

• Filter::new()

• .limit(limit)

• .kinds(vec![1, 42])

• .pubkeys([Pubkey::from_hex(pubkey_hex.as_str()).unwrap()].into()) +}

• fn send_initial_filters(damus: &mut Damus, relay_url: &str) { info!("Sending initial filters to {}", relay_url); let mut c: u32 = 1; @@ -452,6 +471,7 @@ impl Damus { let mut timelines: Vec = vec![]; let initial_limit = 100; let queries_json_path = "queries/global.json";

• let mut initial_pubkey: Option = None;

  if args.len() > 1 {
      for arg in &args[1..] {

  @@ -460,21 +480,34 @@ impl Damus { } } else if Path::new(queries_json_path).exists() { let file_content = fs::read_to_string(queries_json_path).expect("Failed to read file");

• let filter = serde_json::from_str(&file_content).expect("Failed to deserialize");

• let filter: Vec = serde_json::from_str(&file_content).expect("Failed to deserialize");

• initial_pubkey = filter.iter()

• .filter_map(|f| f.pubkeys.as_ref())

• .flat_map(|pubkeys| pubkeys.iter())

• .next()

• .and_then(|pubkey| PublicKey::from_hex(pubkey.hex()).ok());

      timelines.push(Timeline::new(filter));

•     //vec![get_home_filter(initial_limit)]

• } else {

• panic!("No timelines to load.");

• };

• }

  let imgcache_dir = data_path.as_ref().join("cache/img");
  std::fs::create_dir_all(imgcache_dir.clone());

• let login_state = initial_pubkey

• .map(|key| {

• let keys = Keys::from_public_key(key);

• LoginState::AcquiredLogin(keys)

• })

• .unwrap_or_else(|| LoginState::LoggingIn(LoginManager::new()));

• let mut config = Config::new();
  config.set_ingester_threads(2);
  Self {
      state: DamusState::Initializing,

• login_state, pool: RelayPool::new(), img_cache: ImageCache::new(imgcache_dir), note_cache: HashMap::new(), @@ -935,6 +968,44 @@ fn render_damus_desktop(ctx: &egui::Context, app: &mut Damus) { }); }

+fn account_login_panel(ctx: &egui::Context, login_manager: &mut LoginManager) {

• main_panel(&ctx.style()).show(ctx, |ui| {
• ui.allocate_ui_with_layout(
• egui::vec2(ctx.screen_rect().width(), ctx.screen_rect().height()),
• Layout::from_main_dir_and_cross_align(
• egui::Direction::LeftToRight,
• egui::Align::Center,
• ),
• |ui| {
• ui.add(
• egui::TextEdit::singleline(&mut login_manager.login_key)
• .hint_text("Enter login key"),
• );
• if ui.button("Submit").clicked() {
• login_manager.promise = Some(perform_key_retrieval(&login_manager.login_key));
• }
• if login_manager.promise.is_some() {
• ui.add(egui::Spinner::new());
• }
• if let Some(error_key) = &login_manager.key_on_error {
• if login_manager.login_key != *error_key {
• login_manager.error = None;
• login_manager.key_on_error = None;
• }
• }
• if let Some(err) = &login_manager.error {
• ui.horizontal(|ui| {
• match err {
• LoginError::InvalidKey => ui.label(RichText::new("Invalid key.").color(Color32::RED)),
• LoginError::Nip05Failed(e) => ui.label(RichText::new(e).color(Color32::RED))
• }
• });
• }
• },
• );
• }); +}

• fn postbox(ui: &mut egui::Ui, app: &mut Damus) { let _output = egui::TextEdit::multiline(&mut app.compose) .hint_text("Type something!") @@ -982,7 +1053,29 @@ impl eframe::App for Damus {

  #[cfg(feature = "profiling")]
  puffin::GlobalProfiler::lock().new_frame();

• update_damus(self, ctx);
• render_damus(self, ctx);
• if let LoginState::LoggingIn(login_manager) = &mut self.login_state {
• account_login_panel(ctx, login_manager);
• if let Some(promise) = &mut login_manager.promise {
• if promise.ready().is_some() {
• if let Some(promise) = login_manager.promise.take() {
• match promise.block_and_take() {
• Ok(key) => {
• self.timelines.push(Timeline::new(vec!(get_filter_for_pubkey(100, key.public_key().to_hex()))));
• self.login_state = LoginState::AcquiredLogin(key);
• }
• Err(e) => {
• login_manager.error = Some(e);
• login_manager.key_on_error = Some(login_manager.login_key.clone());
• },
• };
• }
• }
• }
• } else {
• update_damus(self, ctx);
• render_damus(self, ctx);
• } } } diff --git a/src/lib.rs b/src/lib.rs index 05e1be6..3e7a8fe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,6 +19,7 @@ mod timeline; mod colors; mod profile; mod key_parsing; +mod login_manager;

[cfg(test)]

[macro_use]

diff --git a/src/login_manager.rs b/src/login_manager.rs new file mode 100644 index 0000000..08ab1d5 --- /dev/null +++ b/src/login_manager.rs @@ -0,0 +1,23 @@ +use crate::key_parsing::LoginError; +use nostr_sdk::Keys; +use poll_promise::Promise; + +/// Helper storage object for retrieving the plaintext key from the user and converting it into a +/// nostr-sdk Keys object if possible. +pub struct LoginManager {

• pub login_key: String,
• pub promise: Option<Promise<Result<Keys, LoginError>>>,
• pub error: Option,
• pub key_on_error: Option +}

• +impl LoginManager {

• pub fn new() -> Self {
• LoginManager {
• login_key: String::new(),
• promise: None,
• error: None,
• key_on_error: None
• }
• } +}

[jb55]

I've pulled in the following commits from your PR since they looked ok to me for now!

7a113825dd77 Add login key parsing d8fcc573f922 Add nostr-sdk dependency c932efba40b7 update cargo.lock to reflect toml change

Feel free to rebase!

[kernelkind]

I replied to these comments over email, unfortunately they don't get synced to github. I'll probably just reply in github next time if that's ok

[jb55]

On Tue, Mar 26, 2024 at 02:11:34PM GMT, kernelkind wrote:

d8fcc573f922 Add nostr-sdk dependency

Oh I'm confused, it sounded like you don't want to use nostr-sdk but you added this anyway? Do you want me to redo it without the nostr-sdk dependency? By creating my own SecretKey and Keys structs

yes I merge stuff so we can make forward progress. When I'm giving my review I sometimes just state my concerns but will merge it anyway because it's too early in the project to worry about such things. We can just refactor it if it becomes an issue.

I would have explicitly told you to rewrite something if I want it to be rewritten. I'm not always right or have the full motivations as to why you needed the dependency. I will naturally undo anything that is causing too much friction anyways.

Cheers,

Will

[jb55]

On Tue, Mar 26, 2024 at 08:35:25AM GMT, kernelkind wrote:

I replied to these comments over email, unfortunately they don't get synced to github. I'll probably just reply in github next time if that's ok

you can do whatever you want, I receive github comments in my inbox. github-delivered comments have a @.*** email which I use to BCC, so both our mailing list and github comments receive it.