Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Direct message #11

Merged
merged 9 commits into from
Feb 24, 2023
Merged

Direct message #11

merged 9 commits into from
Feb 24, 2023

Conversation

arkanoider
Copy link
Collaborator

Hi @grunch !

Finally did some work on get direct message command, code is full of duplication and needs to be refined.
Surely as you pointed out we need to spawn requests to relays to avoid long time execution.
Table answer for direct message need to have also time inside, will provide it! Also i put a totally static "since time" of an hour. So you cannot see previous message, maybe using a flag for a since time could be useful?
Anyway at least is working now.

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

Great work, there are some things to change before merge this PR, and also please before push run cargo fmt and cargo clippy

Cargo.toml Outdated Show resolved Hide resolved
src/lightning/mod.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@arkanoider
Copy link
Collaborator Author

Yes @grunch ! Will do asap!

@grunch grunch self-requested a review February 22, 2023 13:03
Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

Hey @arkanoider thanks for this PR, still the build if failing, can you check on All checks have failed details link?

@arkanoider
Copy link
Collaborator Author

Hi @grunch i committed something yesterday, just to don't lose work. I know it's not compiling, workin on it. I think that github automatically send you a notification if I commit on the same branch.

@grunch
Copy link
Member

grunch commented Feb 22, 2023

Yes github sent me the notification, you can change this PR to draft until is ready for review 😃

@arkanoider
Copy link
Collaborator Author

Hi @grunch give a look...
Now requests are spawned, seems faster. Added since flag to dm ( minutes ).
Think I have to see if it's possible to help on subcommands flags with clap.

Let me know!

Cosmetics for clap help on subcommands flags
Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

wow this is something!! LGTM!!! thanks buddy

@grunch grunch merged commit 07308fd into MostroP2P:main Feb 24, 2023
@arkanoider
Copy link
Collaborator Author

wow this is something!! LGTM!!! thanks buddy

Thanks @grunch it's an honor for me...and it's my small contribution to bitcoin and satoshi.
Moving on to next commands...easier imo. I will look also Mostro side if there's something to work on.

@arkanoider arkanoider deleted the direct_message branch February 24, 2023 20:23
@grunch
Copy link
Member

grunch commented Feb 24, 2023

take a look on the issues I created 😀

@grunch grunch mentioned this pull request Feb 28, 2023
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.

2 participants