search

Neptune-Crypto / neptune-core

anonymous peer-to-peer cash
Apache License 2.0
23 stars 7 forks source link

Remove emojihash from Neptune Dashboard Overview #135

Closed zkclay closed 2 months ago

zkclay commented 3 months ago

This is my first contribution to Neptune that addresses Issue #26

Before Change

Screenshot from 2024-04-08 16-02-36

After Change Image

Screenshot from 2024-04-10 12-50-47

I didn't quite follow in the code why the default value of of the timestamp is July 1, 2024, or perhaps value this is explained by some issue syncing to the blockchain on my machine, I am open to feedback on this.

dan-da commented 3 months ago

hi @zkclay. Welcome, and thanks for the PR!

The change looks fine/good and it seems to meet the requirements of #26 as stated.

What #26 omitted is that our dependency crate twenty-first has removed emojihash entirely in v0.39, and there are several other places in neptune-core that use it. I count 9 files. So those references will need to be removed as well.

If you feel comfortable doing that, it could be a good exercise for you to familiarize with the codebase a bit more. If not, I think we could merge this PR as-is, and then I can follow up.

Sword-Smith commented 3 months ago

The tip digest should still be shown, just not emojified :)

zkclay commented 3 months ago

@dan-da @Sword-Smith Thanks for taking a look! I will work on making these changes

zkclay commented 3 months ago

@dan-da @Sword-Smith I have made the requested changes, could you please take a look when you have a chance?

Sword-Smith commented 2 months ago

Looks good. LGTM.

Sword-Smith commented 2 months ago

Thanks for the PR. I should probably have squashed the commits before merging though. Oh, well.

zkclay commented 2 months ago

@Sword-Smith Thanks, this is exciting! I'll look around for something else to pick up

Sword-Smith commented 2 months ago

Hope to see more PRs from you in the future :)