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

fix: honour --ttl flag in 'ipfs name publish' #9471

Merged
merged 2 commits into from
Jan 27, 2023
Merged

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Dec 7, 2022

It turns out that the option --ttl in ipfs name publish was never being used, and all IPNS records were being published with a TTL of 0. This was happening because the TTL was being passed to a different package through context, but because types are different (and context is not designed for that), the value would never get to namesys.

I updated the required packages to add the options and consolidate Publish and PublishWithEOL into a single function with optional options. See linked PRs.

@hacdias hacdias marked this pull request as ready for review December 7, 2022 15:23
@hacdias hacdias requested a review from lidel as a code owner December 7, 2022 15:23
@hacdias hacdias self-assigned this Dec 7, 2022
@hacdias hacdias force-pushed the fix/use-ttl-on-ipns-entry branch 3 times, most recently from 1697988 to 6671993 Compare December 8, 2022 10:07
@hacdias hacdias changed the title fix: name publish --ttl option is actually used fix: honour --ttl flag in 'ipfs name publish' Dec 8, 2022
@hacdias hacdias requested a review from Jorropo December 8, 2022 10:55
@hacdias hacdias force-pushed the fix/use-ttl-on-ipns-entry branch 2 times, most recently from f0b9e90 to 1db6669 Compare December 12, 2022 12:03
test/sharness/t0100-name.sh Outdated Show resolved Hide resolved
core/commands/name/name.go Outdated Show resolved Hide resolved
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

@hacdias Thank you for fixing this and indulging my idea to implement ipfs name inspect :)

I've bubbled up all changes, and switched this PR to go-namesys release, but there are some UX issues around ipfs name inspect that should be easy to fix, and better to do it now as we may not have better oportunity later.

See comments below, I'll do another pass on this when I'm back on Thursday 🙏

test/sharness/t0100-name.sh Outdated Show resolved Hide resolved
core/commands/name/name.go Outdated Show resolved Hide resolved
core/commands/name/name.go Outdated Show resolved Hide resolved
core/commands/name/name.go Outdated Show resolved Hide resolved
core/commands/name/name.go Outdated Show resolved Hide resolved
core/commands/name/name.go Outdated Show resolved Hide resolved
core/commands/name/name.go Outdated Show resolved Hide resolved
core/commands/name/name.go Outdated Show resolved Hide resolved
test/sharness/t0100-name.sh Outdated Show resolved Hide resolved
test/sharness/t0100-name.sh Show resolved Hide resolved
@hacdias hacdias requested a review from lidel January 25, 2023 08:55
@hacdias hacdias force-pushed the fix/use-ttl-on-ipns-entry branch 2 times, most recently from 204ea6f to 8b48923 Compare January 26, 2023 16:19
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thank you @hacdias!

core/commands/name/name.go Show resolved Hide resolved
@lidel lidel merged commit 94e7f79 into master Jan 27, 2023
@lidel lidel deleted the fix/use-ttl-on-ipns-entry branch January 27, 2023 01:33
@lidel lidel mentioned this pull request Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants