Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

feat: use PublishOptions for publishing IPNS records #35

Merged
merged 2 commits into from
Jan 24, 2023

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Dec 7, 2022

Requires ipfs/interface-go-ipfs-core#94:

  • Reviewed & merged
  • Dependency updated here

This PR allows us to fix --ttl flag during IPNS record publishing.
See ipfs/kubo#9471

@hacdias hacdias force-pushed the feat/publish-options branch 4 times, most recently from 6436cc9 to eeacbd1 Compare December 7, 2022 14:14
@hacdias hacdias marked this pull request as ready for review December 7, 2022 15:17
@hacdias hacdias requested a review from lidel December 7, 2022 15:18
@hacdias hacdias self-assigned this Dec 7, 2022
@hacdias hacdias requested a review from Jorropo December 8, 2022 10:55
@hacdias hacdias force-pushed the feat/publish-options branch from eeacbd1 to 6b648ff Compare December 12, 2022 11:58
@hacdias hacdias force-pushed the feat/publish-options branch from 6b648ff to 75d18a4 Compare January 24, 2023 09:01
@hacdias hacdias force-pushed the feat/publish-options branch from 75d18a4 to 5fa05c8 Compare January 24, 2023 09:03
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.

Merging, so we can bubble up to ipfs/kubo#9471

@lidel lidel merged commit 3f6313c into master Jan 24, 2023
@lidel lidel deleted the feat/publish-options branch January 24, 2023 23:34
Comment on lines +292 to +297
// This is a bit hacky. We do this because the EOL is based on the current
// time, but also needed in the end of the function. Therefore, we parse
// the options immediately and add an option PublishWithEOL with the EOL
// calculated in this moment.
publishOpts := opts.ProcessPublishOptions(options)
options = append(options, opts.PublishWithEOL(publishOpts.EOL))
Copy link
Member

@lidel lidel Jan 24, 2023

Choose a reason for hiding this comment

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

💭 I think this is still way better than the old context hackery, and quite futrure-proof (allows us to add more opts if needed, without having to change public API)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants