Skip to content

Conversation

@matt3play
Copy link

The manifest does not include versions in the package names, but then when checking the manifest against the state of the application, version names are included. This results in a bunch of "X is in the application but not the manifest" (and the opposite) errors.

This change results in the version being stored the manifest now.

The manifest does not include versions in the package names, but then when checking the manifest against the state of the application, version names are included.  This results in a bunch of "X is in the application but not the manifest" (and the opposite) errors.

This change results in the version being stored the manifest now.
@matt3play
Copy link
Author

I signed the agreement!

@CLAassistant
Copy link

CLAassistant commented Mar 3, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@jasonrclark
Copy link
Contributor

Hey @matt3play! Sorry for the super long delays here. I'm finally getting back to changes on this project and hope to get this tested and cut this week.

Given the change in behavior I'm thinking this should be a 3.0, and maybe I'll cut a pre release for it we can try out first. What do you think?

@matt3play
Copy link
Author

I was viewing this as a bug fix rather than a change in behavior. I suspect no one is depending on this behavior as it simply doesn't make sense (papers tells you that you're missing 100% of your NPM packages from the manifest). I'd only do it as a pre-release if I was being super-cautious.

@jasonrclark
Copy link
Contributor

Interesting, because I do have usages of this that don't have versions listed with npm packages but it doesn't fail for me as you're describing. It works with your update but generates a lot of changes, hence why I was thinking it was a bit more major to alter.

Any chance you've got a repro'ing case that I could see since I'm not getting the same failure? I'm planning to take the change, but would be nice to get to the bottom of what's different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants