PeachCloud
/
peach-workspace
3
2
Fork 1
Code Issues 17 Pull Requests Projects Releases Wiki Activity

Add publish-address function to peach-config #130

Merged
notplants merged 11 commits from publish-address into main 2022-07-11 10:25:16 +00:00
Conversation 4 Commits 11 Files Changed 6 +70 -7
notplants commented 2022-07-06 10:37:43 +00:00
Owner
Copy Link

Based on this issue #127 (comment),

this PR adds a command to peach-config:

peach-config publish-address -a host:port

that creates a Pub address message with go-sbot,
which gossips the address to peers,
and is used for inserting into generating invites

Based on this issue https://git.coopcloud.tech/PeachCloud/peach-workspace/issues/127#issuecomment-13560, this PR adds a command to peach-config: ```peach-config publish-address -a host:port``` that creates a Pub address message with go-sbot, which gossips the address to peers, and is used for inserting into generating invites
notplants added 2 commits 2022-07-06 10:37:44 +00:00
notplants added 1 commit 2022-07-06 10:40:04 +00:00
continuous-integration/drone/pr Build is failing Details
f1ab2caa08 Fix golgi imports
notplants commented 2022-07-06 10:47:51 +00:00
Author
Owner
Copy Link

This PR depends on this PR in golgi to give access to PubAddress:
golgi-ssb/golgi#50

cargo-clippy will continue to fail here in the CI until this golgi PR is merged

This PR depends on this PR in golgi to give access to PubAddress: https://git.coopcloud.tech/golgi-ssb/golgi/pulls/50 cargo-clippy will continue to fail here in the CI until this golgi PR is merged
notplants requested review from glyph 2022-07-06 10:48:44 +00:00
glyph requested changes 2022-07-06 12:32:12 +00:00
glyph left a comment
Owner
Copy Link

Looks good to me overall. I just have one requested change related to subcommand naming.

Looks good to me overall. I just have one requested change related to subcommand naming.
peach-config/src/main.rs
@ -58,0 +59,4 @@
/// Publish domain and port.
/// It takes an address argument of the form host:port
#[structopt(name = "publish-address")]
glyph commented 2022-07-06 12:31:10 +00:00
Owner
Copy Link

I recommend picking a single convention for multi-word flags to keep the CLI API consistent. There is changepassword and now publish-address. I don't have a strong preference (either no space or hyphen).

I recommend picking a single convention for multi-word flags to keep the CLI API consistent. There is `changepassword` and now `publish-address`. I don't have a strong preference (either no space or hyphen).
notplants commented 2022-07-07 08:40:13 +00:00
Author
Owner
Copy Link

agreed, will change both to use hyphens

agreed, will change both to use hyphens
👍 1
glyph marked this conversation as resolved
notplants added 1 commit 2022-07-07 08:41:57 +00:00
continuous-integration/drone/pr Build is failing Details
bdac23092a Change changepassword to change-password
notplants added 1 commit 2022-07-07 08:50:05 +00:00
continuous-integration/drone/pr Build is failing Details
58bf306d3b Fix golgi import
notplants added 1 commit 2022-07-07 08:51:55 +00:00
continuous-integration/drone/pr Build is failing Details
7daab74b37 Fix golgi import
notplants added 2 commits 2022-07-07 08:57:47 +00:00
continuous-integration/drone/pr Build is failing Details
90badbfe30 Add cargo.lock to .gitignore
notplants added 1 commit 2022-07-07 11:42:42 +00:00
continuous-integration/drone/pr Build is passing Details
466db8ceea Update sbot.rs to new history_stream api
notplants commented 2022-07-07 11:47:22 +00:00
Author
Owner
Copy Link

@glyph in order to make this code compile, I needed to delete the Cargo.lock file (which we wanted to get rid of anyway) so that it would actually use the latest version of golgi (and not the version that was in Cargo.lock). Once using the latest golgi, it unearthed a small mismatch in peach-web where it was still using the old history stream api before your update to golgi (from SsbMessageValue to SsbMessageKVT), so I have also included a fix to this in peach-web in this PR, which maps the vec of SsbMessageKVT to a vec of SsbMessageValue

@glyph in order to make this code compile, I needed to delete the Cargo.lock file (which we wanted to get rid of anyway) so that it would actually use the latest version of golgi (and not the version that was in Cargo.lock). Once using the latest golgi, it unearthed a small mismatch in peach-web where it was still using the old history stream api before your update to golgi (from SsbMessageValue to SsbMessageKVT), so I have also included a fix to this in peach-web in this PR, which maps the vec of SsbMessageKVT to a vec of SsbMessageValue
glyph added 2 commits 2022-07-07 14:55:15 +00:00
glyph commented 2022-07-07 15:00:21 +00:00
Owner
Copy Link

@notplants

I hope you don't mind me pushing to your PR 🙏

Two changes I've made:

  1. Remove Cargo.lock from .gitignore. We want to keep the lockfile in peach-workspace to ensure that all crates are using the same version of all dependencies. golgi was a different story, since it's a standalone library (and therefore should not have the lockfile included in the repo).

  2. I've removed the .map you added to transform the vec of SsbMessageKVT to a vec of SsbMessageValue. Instead, I just queried the kvt directly by adding .value (msgs[0].value.sequence). This way should be more performant. Sorry you had to bump into this mismatch...I didn't think about that when making the change.

I'm still waiting for the CI to complete but it should be good to go soon :)

@notplants I hope you don't mind me pushing to your PR 🙏 Two changes I've made: 1) Remove `Cargo.lock` from `.gitignore`. We want to keep the lockfile in `peach-workspace` to ensure that all crates are using the same version of all dependencies. `golgi` was a different story, since it's a standalone library (and therefore should not have the lockfile included in the repo). 2) I've removed the `.map` you added to transform the vec of `SsbMessageKVT` to a vec of `SsbMessageValue`. Instead, I just queried the `kvt` directly by adding `.value` (`msgs[0].value.sequence`). This way should be more performant. Sorry you had to bump into this mismatch...I didn't think about that when making the change. I'm still waiting for the CI to complete but it should be good to go soon :)
👍 1
glyph commented 2022-07-07 15:11:03 +00:00
Owner
Copy Link

🟢

🟢
notplants merged commit 03a0a51f4d into main 2022-07-11 10:25:16 +00:00
notplants deleted branch publish-address 2022-07-11 10:25:20 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
glyph
Labels
Clear labels
  
bug

Something is not working

  
documentation

   
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
maintenance

Pruning and weeding

  
peach-lib

   
peach-network

   
peach-oled

   
peach-stats

   
peach-web

   
question

More information is needed

  
refactor

Code rewrite

  
wontfix

This won't be fixed

No Label
bug
documentation
duplicate
enhancement
help wanted
invalid
maintenance
peach-lib
peach-network
peach-oled
peach-stats
peach-web
question
refactor
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: PeachCloud/peach-workspace#130
No description provided.
Delete Branch "publish-address"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?