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

[WIP] feat: default preload false in ci #1819

Closed
wants to merge 3 commits into from

Conversation

alanshaw
Copy link
Member

BREAKING CHANGE: Content preloading on preload nodes is now disabled by default in test and CI environments. This can be overridden by explicitly enabling preloading in test/CI by setting preload.enabled to true in the IPFS constructor args.

resolves #1815

cc @eefahy

@ghost ghost assigned alanshaw Jan 15, 2019
@ghost ghost added the status/in-progress In progress label Jan 15, 2019
Alan Shaw added 3 commits January 15, 2019 12:26
resolves #1815

License: MIT
Signed-off-by: Alan Shaw <[email protected]>
License: MIT
Signed-off-by: Alan Shaw <[email protected]>
License: MIT
Signed-off-by: Alan Shaw <[email protected]>
@alanshaw alanshaw force-pushed the feat/default-preload-false-in-ci branch from bdf1efc to 6709244 Compare January 15, 2019 12:27
Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

Couldn't we configure this as part of the build environment or during the test suite setup instead of changing the application code to have test-specific code paths?

@alanshaw
Copy link
Member Author

We could, but this also mitigates against preload happening when run in CI by people using js-ipfs in their applications.

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.

👍 from me. I don't necessarily agree that this should be classified as a breaking change. The users of this module will have to do zero changes to their codebase and there won't be any behavior change for them. That said, I'm also not opposed if you feel strongly about it :)

@alanshaw
Copy link
Member Author

The users of this module will have to do zero changes to their codebase

In production, no, but if users have integration tests that rely on two nodes exchanging content via the preload nodes then they'll need to explicitly set preload to true.

What is "breaking" anyway? 😆

@alanshaw alanshaw mentioned this pull request Jan 29, 2019
50 tasks
@alanshaw alanshaw changed the title feat: default preload false in ci [WIP] feat: default preload false in ci Feb 11, 2019
@alanshaw
Copy link
Member Author

...need to fix the browser issues before this can be merged

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

Successfully merging this pull request may close these issues.

Disable preloading in test environments
3 participants