Make CI green again

[victorb]

Currently, master has not had a passing build since 6 of April (https://ci.ipfs.team/job/IPFS/job/js-ipfsd-ctl/job/master/) and none of the PRs are having green builds right now.

Since js-ipfsd-ctl is a integral part in our development and testing, we have to make the tests pass as long as the code is correct. Having flaky tests is a deal breaker, as we start ignoring the test results.

I'm not sure why the builds are not passing, but we should stop adding new features or fixes until we can trust CI, as otherwise we cannot safely merge changes.

[dryajov]

Agree. Now that most changes related to class-is have been bubbled, it should be green again. Looking into it now.

[dryajov]

After investigating, in order to make CI green again, we need:

• Issue introduced on 0.20.1 js-ipfs-repo#165 fixed
  • but meanwhile lock it down to 0.20.0, PR here chore: update ipfs-repo #241
• fix: change ^ to ~ on 0.x.x deps fix: change ^ to ~ on 0.x.x deps js-ipfs#1345

[victorb]

@dryajov seems we could already open a PR and manually override the node modules to at least see if makes it green, correct?

[dryajov]

@victorbjelkholm We can, but that would still break for our users, wont it?

I think what we need is to release a hot fix js-ipfs with the locked js-ipfs-repo version to 0.20.0. The reason this is all broken now, is because we used ^ for 0.x.x versions, and broke semantic versioning in js-ipfs - this is now broken for everybody not just ipfsd-ctl. I think this is pretty bad, but fixing it just for the CI won't make this issue go away.

[dryajov]

Ah, I take that back @victorbjelkholm - we only need to pin it down here, js-ipfs is fine as it is still on 0.18.x...

[dryajov]

The ^ vs ~ issue still stands, as we're wrongly using ^ for ipfs-version here - https://github.com/ipfs/js-ipfsd-ctl/blob/master/package.json#L89, which forces it to pick up the latest, ipfs-repo version 0.21.0. We need to pin it to ~0.20.0.

[victorb]

@dryajov yeah, I just wanted to confirm that what we think would solve the issue, actually solves it (reproduce it externally so we can be 100% nothing more needs to get done).

Speaking about ^ vs ~, is there a reason we don't just specify the versions as-is, without any version ranges? Having dependencies automatically switch out from under you feels like a bad default. Way easier to just remember "no version ranges" than "Use ~ for everything below 1.0.0 and ^ for everything above 1.0.0" as well. We could also enforce this via CI if we'd wish (even the ~/^ rule could be enforced)

[dryajov]

Yeah, I agree, but I would suggest we just use lock files, since thats what they were made for - it would make sense to just lock to a version had they not been introduced.

[victorb]

@dryajov lock files would be nice. Would be even nicer with lock files that works both for yarn (I know many of the devs that uses yarn (and me) + CI uses yarn) and npm (probably most people still using npm?), since they have different ones. Locking the version directly in package.json works for both though.

[dryajov]

good point, forgot about incompatible lock files 😭

[daviddias]

@vasco-santos now that js-ipfs has been released, you can updates and get CI green again.

[vasco-santos]

@diasdavid CircleCI is not green yet!

Last time it was green was in @dryajov PR. Dmitriy do you know what is wrong now?

[dryajov]

The failures come from the version logic:

There is a bug in js-ipfs introduced with https://github.com/ipfs/js-ipfs-repo/pull/164/files#diff-1fdf421c05c1140f6d71444ea2b27638R216. Changing the error message broke version handling, which ignores uninitialized repos and sources the version directly from the package if no repo exists. Here is a fix ipfs/js-ipfs#1374.

[vasco-santos]

Thanks for your analyzes @dryajov !

[alanshaw]

@dryajov I'm confused, https://github.com/ipfs/js-ipfs-repo/pull/164/files#diff-1fdf421c05c1140f6d71444ea2b27638R216 didn't change the error message

[vasco-santos]

CI is green again!! 🙌
Thank you all