Skip to content
This repository was archived by the owner on Feb 12, 2024. It is now read-only.

Conversation

@hugomrdias
Copy link
Member

@hugomrdias hugomrdias commented Jun 4, 2020

This PR makes ipfs config show work again and config.get first arg required.

Also this makes ipfsd-ctl work again.

This PR `ipfs config show` work again and makes `config.get` first arg required
@hugomrdias hugomrdias requested a review from achingbrain June 4, 2020 13:49
@achingbrain
Copy link
Member

Can we change the API to be something like:

ipfs.config.getAll()
ipfs.config.get(key)

instead of

ipfs.config.get(undefined)
ipfs.config.get(key)

Isomorphism in js is such a hack.

@hugomrdias
Copy link
Member Author

Maybe ipfs.config.show to keep it consistent with http api and cli ?

@achingbrain
Copy link
Member

The CLI then literally shows the config though, which is not what we are doing, and the HTTP API is generated from the CLI commands.

I think we can give the API commands more straightforward names, though they may call HTTP API endpoints that are named less descriptively.

@achingbrain
Copy link
Member

Could do ipfs.config.read? Then it's similar to ipfs.files.read?

achingbrain added a commit that referenced this pull request Jun 5, 2020
Tactical fix to get the release out while we solve this properly
in #3067
achingbrain added a commit that referenced this pull request Jun 5, 2020
Tactical fix to get the release out while we solve this properly
in #3067
achingbrain added a commit that referenced this pull request Jun 5, 2020
Tactical fix to get the release out while we solve this properly
in #3067
@hugomrdias
Copy link
Member Author

Makes sense, lets go with getAll its already there in ipfs-repo and also other apis i researched use the same method.

@hugomrdias
Copy link
Member Author

closed in favor of #3071

@hugomrdias hugomrdias closed this Jun 7, 2020
@hugomrdias hugomrdias deleted the fix/config-show branch June 7, 2020 19:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants