Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Check package json version instead of zos version for dependencies #309

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

spalladino
Copy link
Contributor

Changes all dependency version validations to check against the package.json version instead of the zos.json version. This allows publishing an update of the npm package without modifying the zos.json versions. The downside is that we are coupling the zos.json declared dependency with npm versioning (the dependencies node of the zos.json file will now list dependencies with their npm version, and not their zos version), but we can review this in the future when we allow other package managers to be used. We can discuss whether this is the best approach though, as it allows to completely unsync the npm version and the zos version for a dependency, and whether to merge it now in case it introduces potential new issues.

We do use the zos.json version to check the zos.network.json files. If a dependency updates its zos.json version, but fails to update a zos.network.json, the consumer will reject using the dependency on that network.

Fixes #308

@facuspagnuolo
Copy link
Contributor

We already discussed this offline, but I'm dropping my comment here anyway. I don't like the idea of coupling the package.json version to the zos.json. I'd prefer working on some improvements that prevent the user to get those files unsynched, additionally, we should reconsider the versions validations we are doing to see if there is a way to handle this scenario improving the user experience.

@facuspagnuolo facuspagnuolo added the status:on-hold Do not move forward for now label Oct 24, 2018
@facuspagnuolo facuspagnuolo removed their assignment Jan 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:on-hold Do not move forward for now
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants