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

Add Decoder Extension - Update extensions.json #383

Merged
merged 11 commits into from
Sep 10, 2024

Conversation

bitkarrot
Copy link
Collaborator

add decoder extension

@motorina0
Copy link
Collaborator

The functionality is useful, but should it be an extension?

@dni dni self-requested a review July 24, 2024 08:47
@bitkarrot
Copy link
Collaborator Author

its tiny. I made it because i didn't want to bother you guys and it was convenient. If you want to roll into core, that's cool by me too.

@motorina0 @dni

@dni
Copy link
Member

dni commented Jul 25, 2024

added a pr to the extensions, after this i am fine with merging it.

bitkarrot/decoder#1

can you release and use a version with v, like v0.0.2

@bitkarrot
Copy link
Collaborator Author

@dni here is the latest with your PR merged, lmk if that is ok

https://github.com/bitkarrot/decoder/releases/tag/v0.0.5

@motorina0
Copy link
Collaborator

@bitkarrot just an idea, do you think this extension would be more accessible if it was served as a public page?
For example: https://demo.lnbits.com/decoder/check

Is there any reason for requesting a user login?

@bitkarrot
Copy link
Collaborator Author

Agree we should make the page accessible as a public page, similar to https://lightningdecoder.com/

where is the user login? its just an extension, there should be no login required? @motorina0

@bitkarrot just an idea, do you think this extension would be more accessible if it was served as a public page? For example: https://demo.lnbits.com/decoder/check

Is there any reason for requesting a user login?

@bitkarrot
Copy link
Collaborator Author

About the public page - it might be good to make this an opt in or out, as a bot could come along and just spam the input box relentlessly.

@bitkarrot
Copy link
Collaborator Author

Had a brief chat with @motorina0

we're just gonna leave the decoder extension as-is. If there is nothing else you need fixed, please to add?

extensions.json Outdated Show resolved Hide resolved
@motorina0
Copy link
Collaborator

CI failing at check-json

@bitkarrot
Copy link
Collaborator Author

CI failing at check-json

fixed check-json but jmeter is failing, also fails on lncalendar pr, unclear why

1722580508975,1440,Fetch vetted extension list,404,Not Found,Thread Group 1-1,text,false,"Test failed: code expected to equal /

@motorina0
Copy link
Collaborator

CI failing at check-json

ah, its because the PR is from a different repo (bitkarrot:patch-1), but it tries to find the branch on this repo

https://raw.githubusercontent.com/lnbits/lnbits-extensions/patch-1/extensions.json

@dni can you please have a look

@motorina0
Copy link
Collaborator

All good except the CI fix which is not related to this PR, but to the fact that an external repo is used.

@bitkarrot
Copy link
Collaborator Author

updated branch LFG!!

Copy link
Contributor

@talvasconcelos talvasconcelos left a comment

Choose a reason for hiding this comment

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

I'd bet we had decode endpoint in the core API...

@bitkarrot bitkarrot merged commit 369e31b into lnbits:main Sep 10, 2024
2 of 3 checks passed
@bitkarrot
Copy link
Collaborator Author

Whooops i just squashed and merged, i thought it was just merge :O

@motorina0
Copy link
Collaborator

Whooops i just squashed and merged, i thought it was just merge :O

All good! Congrats on the extension!

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.

4 participants