Skip to content

Re-add package.json #575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 7, 2022
Merged

Conversation

jdesrosiers
Copy link
Member

The package.json file is necessary so I can include the test suite as an npm dependency in my implementation. It was recently removed and broke several of my packages.

@Julian
Copy link
Member

Julian commented Jul 6, 2022

Do you have any desire to help maintain the npm specific test suite repository?

The package.json that was here wasn't really added to support the way it sounds like you were using it -- it was originally added because Evgeniy mistakenly thought this repo was directly testing my python implementation as part of CI (#314) and therefore wanted to test ajv here too, but that wasn't what was happening here.

Historically though I've tried not to do this, otherwise we'd end up with 15 different package manager files in this repo for every language.

@jdesrosiers
Copy link
Member Author

Do you have any desire to help maintain the npm specific test suite repository?

I didn't know that existed, but even if I did, I wouldn't use it because it's something that needs to be maintained which means it will at best be behind the latest changes for a period of time and at worst abandoned like it is now. By depending on the repo directly, I get the latest test with no extra steps. I'd rather use git submodules and hate git submodules.

In any case, removing package.json broke several of my packages and probably affects others as well. Can we add it back to unbreak things for now? Then we can discuss removing it and if we decide to remove it, give implementations notice and time to update their implementations before removing it.

@Julian
Copy link
Member

Julian commented Jul 7, 2022

Can we add it back to unbreak things for now?

So be it... we don't really have a way to communicate with implementations (other than just removing it), so after merging this it doesn't seem promising to fix the problem, but oh well, I guess I just will continue learning to ignore it even though it unduly gives preference to the JS ecosystem.

By depending on the repo directly, I get the latest test with no extra steps.

FWIW I think this is a bug, not a feature, one related to the adage of "pin your dependencies", but that battle is likely lost too.

@Julian Julian merged commit 26a51ca into json-schema-org:main Jul 7, 2022
@karenetheridge
Copy link
Member

karenetheridge commented Jul 7, 2022

I would suggest a section in the README "how to use this test suite in your application", with subsections for different languages. I can write the perl section -- the test suite is available as an installable distribution as Test::JSON::Schema::Acceptance (authored by @Relequestual and now maintained by me).

@jdesrosiers
Copy link
Member Author

FWIW I think this is a bug, not a feature, one related to the adage of "pin your dependencies"

I disagree in this case. When I'm developing, I want to know if something is wrong as quickly as possible with no extra steps that I might forget to do. Breaking things is the point.

@Julian
Copy link
Member

Julian commented Jul 7, 2022

I would suggest a section in the README "how to use this test suite in your application"

That exists for JS, though it seems it still wasn't seen, so it'd have to be in some other more noticeable way. Suggestions certainly welcome, though I personally don't recommend this way of using the suite, the one "blessed" way for me has always been via either a subtree or subrepo, and I don't want to maintain support for other languages here, so if we do have such a section it's likely to become a graveyard itself of abandoned packages. But I certainly won't get in the way of such a PR adding docs.

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