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

Fix js-ipfs daemon config params #914

Merged
merged 2 commits into from
Jul 19, 2017
Merged

Fix js-ipfs daemon config params #914

merged 2 commits into from
Jul 19, 2017

Conversation

sktt
Copy link
Contributor

@sktt sktt commented Jul 18, 2017


I was unable to run tests, linting or anything. Did not work on node 8.1.2 or 6.11.1

  - Respect `--enable-experimental-pubsub`
  - Don't overload repo config with cli args (fixes ipfs#868)
  - Typo
@victorb
Copy link
Member

victorb commented Jul 18, 2017

I was unable to run tests, linting or anything. Did not work on node 8.1.2 or 6.11.1

That seems weird, what npm version? Think npm@5 and above is not working well with aegir, but npm@4 should work ok.

Also, CI is failing, unsure of why... @diasdavid ideas?

@sktt
Copy link
Contributor Author

sktt commented Jul 18, 2017

Edit: I broke the tests because I don't pass down config from http-api to core. There's a problem here though: config when using the $ js-ipfs daemon is the cli arguments (pubsub/sharding/etc) but from spawnDaemon in the gulpfile, it's confiuration file parameters.
@victorbjelkholm shouldn't cli config params be passed as a separate argument, ie httpAPI(repo, [[configOverrides], [cliArgs]]), let me know what you think.

Its npm version 5.2.

- Add an extra parameter for `HttpAPI` for `cliArgs`
Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @sktt

@@ -16,7 +16,7 @@ module.exports = {
default: false
},
'enable-pubsub-experiment': {
type: 'booleam',
type: 'boolean',
Copy link
Member

Choose a reason for hiding this comment

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

great catch

@daviddias daviddias merged commit e00b96f into ipfs:master Jul 19, 2017
dryajov pushed a commit that referenced this pull request Sep 1, 2017
* Fix js-ipfs daemon config params

  - Respect `--enable-experimental-pubsub`
  - Don't overload repo config with cli args (fixes #868)
  - Typo

* Treat cli args and config overload as seperate

- Add an extra parameter for `HttpAPI` for `cliArgs`
MicrowaveDev pushed a commit to galtproject/js-ipfs that referenced this pull request May 22, 2020
This was accidentally disabled and no tests existed to catch it.

* [x] depends on ipfs-inactive/interface-js-ipfs-core#420

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
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.

Running daemon in CLI screws up config
3 participants