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

nip19: remove note1. #456

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Conversation

AsaiToshiya
Copy link
Collaborator

@fiatjaf fiatjaf merged commit a8a805f into nbd-wtf:master Nov 19, 2024
1 of 2 checks passed
@AsaiToshiya AsaiToshiya deleted the nip19-remove-note1 branch November 19, 2024 15:03
@eskema
Copy link

eskema commented Nov 19, 2024

this feels very wrong to me

@fiatjaf
Copy link
Collaborator

fiatjaf commented Nov 19, 2024

Why?

@eskema
Copy link

eskema commented Nov 20, 2024

because the decode function is still needed to read all the past content that is already out there, and there are probably a lot of code that depends on this functions existing to continue working. (at least mine does) It’s one thing to have clients phase it out, but it’s another to just break things in clients that still use it

@fiatjaf
Copy link
Collaborator

fiatjaf commented Nov 20, 2024

He is right, @AsaiToshiya.

@AsaiToshiya
Copy link
Collaborator Author

What about encoding? Is it necessary?

This PR is just a suggestion and I don't mind if you revert changes.

@fiatjaf
Copy link
Collaborator

fiatjaf commented Nov 20, 2024

Encoding we should remove.

@AsaiToshiya
Copy link
Collaborator Author

Ok, I'll open another PR.

@eskema
Copy link

eskema commented Nov 20, 2024

I use note1 encoding for note ids since we moved out of showing hex strings to users a long long time ago, nevent and naddr cannot be used for such which leaves me with no other option than reverting back to hex strings. fun

@fiatjaf
Copy link
Collaborator

fiatjaf commented Nov 20, 2024

Why can't you use nevent? Of course you can and should!

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.

3 participants