-
Notifications
You must be signed in to change notification settings - Fork 354
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
New maintainer required #658
Comments
Hey, I'd be happy to be added as an admin too here, because indeed Composer depends on it and it's the only dependency we have that is not under our control or part of symfony (which has a fairly decent maintenance outlook). I however have close to no knowledge of the internals of this lib, and limited json-schema knowledge, so I am not likely to be able to do a lot of maintenance unless absolutely necessary and worth the time to dive in. |
Thanks @Seldaek - I appreciate that. I think that adding you as an admin would be a good idea if you're OK with it. Even if you won't actually be maintaining it, you're a highly trusted party with an interest in the outcomes, and it would help to reduce the bus factor somewhat. |
@robocoder How familiar are you with the internal workings of this library? I'm not seeing any previous contribution history from you. |
I don't think you can grant admin permissions to other people on a personal repository. So it might make sense to migrate the repository to an organization if @justinrainbow is not active anymore. |
I was started a new Symfony project and need a JSON validation library. From the memory I came here as my colleagues have used this library for validating JSON schemas. Now as there is this unfortunate situation, I'd like to just point alternative(s) to this thread so ppl like me can then decide which way to go. My (short) research found 2 alternatives:
You can check:
Example (me and my project):
All that said, it should be quite trivial to swap the library in the our app if needed. Maybe even create internal wrapper which you then use in your app and can just refactor the wrapper class when changing the library. Lastly: Thanks you @justinrainbow for all the hard work on the library and I wish all the best for you ❤️ |
@justinrainbow granted me access so at least that's one more person able to merge stuff now 👍🏻 What I wrote above still stands though, I am not very familiar with this library's code, but I do have an interest in keeping it alive as long as Composer depends on it. |
Thanks for the update @Seldaek - that is really good to hear. Thanks @justinrainbow also for actioning that 🙂. |
Thank you @Seldaek 🙏 |
Hello Given the problem with the maintenance of this package, I would suggest you mark it as archived and point users to https://github.com/opis/json-schema. It's very close to a drop-in replacement for this and is actively supported, has no issues or open PRs, 2.2m+ downloads and supports PHP 7.4 and 8.0. Thanks for making this though. Been using it for years. |
@nickdnk Does that package support PHP 5.6, and draft-03 schemas? Neither of those things appear to be listed on the README page, and the requirements seem to indicate that PHP7.4 is required. Unfortunately, backwards compatibility is a significant consideration for this library. Composer itself is a major dependant, so we can't just abandon old PHP versions lightly, even though they're no longer officially supported. There are also still a large number of schemas out there which use draft-03 syntax for things. Not as big a deal as the PHP version constraint, but still a consideration. |
The draft support is listed here: https://opis.io/json-schema/2.x/ It does not support PHP 5.6 which nobody should be using; and rightly so anything using PHP 5.6 should be considered legacy/archived code as well. I'm not saying this is going to work as a drop-in for all projects on all platforms, but that for people running on modern platforms and not on unsupported versions of PHP, this is a good replacement. I did not say to remove this package from Packagist, just to suggest using something newer instead of having people set this package up in cases where opis could be used. |
If using PHP 5.6 which is EOL and unsupported then I guess it's OK to use old releases of justinrainbow/json-schema. My understanding is that even if repo is archived, the old releases are available through Packagist. |
Exactly. |
I did not see you had already mentioned this, @back-2-95. |
Even if you don't care about draft-03, remember that Swagger and OpenAPI < 3.1 use [an "extended subset" of] draft-04 |
For Composer we'd probably switch to swaggest/php-json-schema if we switch at all, as it supports PHP 5.4+, and we need at least 7.2+ going forward. 7.4 for opis/json-schema is not workable for us. That said I don't think we have a need for the latest json schema drafts as draft 4 seems to do the job at the moment and I don't expect big changes in our schema. |
I'd like to get this repo up to speed with GH actions, perhaps phpstan, and get things built on PHP 8.1 etc.. But I am quite confused by the branching strategy. It looks like branch 5.x.x is both behind and ahead of master. Branch 6.0.0-dev is pretty much the same as master. Releases appear to have been done from 5.x.x until now, and it has a bunch of backport commits.
If anyone can shine some light on the situation that would be most helpful. I don't want to make even more of a mess :) |
Some more git digging makes me believe master is indeed v6 dev, the 6.0.0-dev branch should probably be merged then deleted. Now my question would be.. what is missing for a 6.0.0 release? Is master not stable? Were there features missing? Could we release it as is and then abandon the 5.x.x branch so we have to stop doing backports (trying to reduce maintenance work here as nobody has time for anything..)? |
Yes. Not all PRs are backwards-compatible with 5.x.x, but those that are, I have been backporting.
Yes.
Many people do work on master (v6). It just doesn't have a published release version, because it isn't finished (there are a few outstanding bugs, and a lack of documentation) - however, functionality-wise it works for most purposes.
It's already been merged, and master has since diverged from it. The 6.0.0-dev branch should be deleted without further merging it; there is nothing in that branch which is not already incorporated into master a long time ago. Previously, 5.x.x was master, and the work was happening in 6.0.0-dev - however, people kept opening PRs against master despite the instructions saying to open against 6.0.0-dev. We moved to the current branch naming in order to resolve this, so that master was actually the correct PR target.
A few outstanding bugfixes, and documentation.
Master (v6) is stable, however there a few specifric situations where it is known to violate the spec. These are the bugs I refer to above.
If you are happy to push out a slightly buggy 6.0.0 release, yes. For what it's worth, the backports aren't usually too much hassle to merge. I usually just run a If you take a look at previous backport PRs, in the description for each I list the PRs merged to master since the previous set of backports, and categorise which were backported, which were skipped (and why), and which weren't applicable to the 5.x.x branch. |
@erayd ok many thanks for putting it all down here in detailed form. It kinda matches what I ended up understanding but it's good to have it documented somewhere. |
Also: I have now deleted the 6.0.0-dev branch to reduce further confusion. |
I've always been fond of the package and recognize the work here that has been done by @justinrainbow and contributors. I would like to offer helping out and see if we can revive the package and help the community. As a maintainer and contributor I know help from others is valuable and needed, But I can't do it by myself not do I want to do this alone. In order to help out I could do:
But that also requires someone/others with permissions in the project to
Is there anyone who still feels like putting in the effort? @erayd @Seldaek @bighappyface pinging you guys a based on the release and comments in the issues it seems you're at least having some permissions to do so. Sorry to be this direct and a clear "No" is also fine for me, but I would like to make sure putting in the effort is at least having a change. |
@DannyvdSluijs Help would very much be appreciated - thank you! I'm chronically short of time, so the biggest bottleneck is likely to be code review. Small changes are easier / faster to digest than big ones, and more likely to get merged. If you know a second person who is highly familiar with the library and has the capacity to take on some of the review load, that would help a lot too. For clarity, while I'm usually able to respond promptly, I don't have the time to do much. If you do choose to contribute here, the reality is you'll be carrying most of the load alone, and anything that requires significant amounts of my time (especially complex code review) is likely to be very delayed. Probably the most helpful thing you could do right now that wouldn't be too bottlenecked is issue / PR triage. If you create a master issue with a list of what needs closing, and what needs to remain open (and possibly tagged), I'm happy to go through the list & action them accordingly. That will then at least clear out the stuff that's no longer applicable, and make it clearer what is left that actually needs attention. |
Thanks for the quick response. That at least shows there is a sign of life for the project and that it at least is worth putting effort into it. I'll try and see if I can first focus on issue cleanup (triage and follow-up on reporters). I'll also see if I can review some of the pending PR's before creating new ones (maybe a couple to help with issues, such as #709) to not overload review process and create rebasing work for contributors. Besides all the missing draft support and issues it also seems this project could benefit from a pipeline revamp. Has there been any discussions about supporting older PHP versions? Based from the packagist.org stats if we would drop PHP <= 7.3 we would only leave 7.4% of the install stuck at the current version and can shake quite some technical debt and help out people on newer versions with some speed improvements due to raising the minimum level and calling more efficient functions. |
Wanted to note that as of April 2024 the picture looks very different: Opis has gone over two years without a new release and has some pretty low-hanging fruit (like opis/json-schema#130) just sitting there in the PRs with no response from the maintainer. I've been using both these libraries for a long time but am leaning toward finally consolidating on this one unless we see some signs of life at Opis. Much appreciation to the people keeping this package moving! (Opis is a great piece of software too, hope to see that resurrected as well.) |
IntroI believe since end of March we've made good progress with the cleanup of old issues and pull requests. I've been trying to see what would be the next steps forward. Which has led me to posting this comment. Next stepsAfter seeing some of the comments on issues and pull requests I've been working on some milestone ideas (A milestone doesn't have the a mayor release) 1. Get version 6.0.0 shipped: This will resolve issue for people that have been reporting them based on older branches. We can limit the support to the last two mayor versions.. I can't do this aloneI'm still really happy to help out and I get the feeling other people as well, there is s positive vibe going on. There is just one key element which is required to make progress, which is a person(s) who can merge pull requests, create new release etc. In my eyes this requires either time from the original authors or a new maintainer is actually appointed which such permissions. And even with just me as an additional maintainer I would still like for someone to pair-review any of my pul requests. I don't have a preference for either option but I feel like we need to pick one and go for it. Otherwise the project will be back to it's limbo state from a couple of months ago. This project has some great potential to grow and just making a fork and steal the thunder from all previous work done here by others just doesn't feel right. QuestionSo @erayd if you could spare maybe ten minutes to tell us how you would see this project move forward that would be a great insight for me and many others as well. Also I don't want to put you on the spot there is no obligation whatsoever, I'm just trying to chase down this issue and push it over the finish line. Thanks for all you work on |
Catching up on this conversation as we're trying to re-evaluate our dependency tree over at Drupal. Has the question of repository ownership/admin been addressed? It looks like there's a critical mass of people on this issue who are interested in keeping things going here, but I imagine there's a practical consideration of maintaining pipelines, the packagist link and such. Has there been any further thought on where this repo should live, perhaps in an organization that has a better succession planning capability? |
@bradjones1 I don't think that's the most pressing matter.. @erayd has admin access iirc, i have push access but not sure if more (can't see that on the mobile github app). Finding people with time and motivation is the more critical part. If at some point we get stuck managing permissions here we can always fork it but hopefully that won't be needed. |
Your next steps look good. I'm still chronically short on time (this comment still very much applies), but happy to chip in where I can. If it would be helpful, you are welcome to direct my focus - I'm happy to spend what little time I have available on wherever you see it being most useful (e.g. if there is a particular PR you want reviewed etc).
Another maintainer who has the time available to work alongside @DannyvdSluijs would really get things moving I think; that way we'd have enough to get proper code review done on significant changes without it dragging on for months or years (as it does when waiting for me). Ideally you want someone who is intimately familiar with the JSON Schema spec, and the differences between its various versions. Many of the problems that people report are simply related to not properly understanding the spec, and various PRs that get proposed also often run into spec-violation issues, even if the code looks good - so they really need signoff from someone who can properly consider the spec implications of what they are looking at. The official test suite is really helpful, but isn't exhaustive, and it's quite possible to merge something that violates the spec, but still passes the test suite. If you find someone good to help out, but are struggling on the spec front, please feel free to ping me for that side of the review. It won't be as good as if the main reviewer has that skillset, but it'll be better than merging stuff without proper spec consideration.
It's a personal repo owned by @justinrainbow - downside of that means that he's the only one with proper admin access, as that cannot be delegated on personal repos. He hasn't been engaged on this repo for many years, but does grant access to it when we need new people added. Longer-term though, it would be better to have this sitting under an org where ownership & succession can be managed properly. One idea that's been discussed in the past has been to convert this into a purely dependent package (thus keeping all the dependency links in place), but move the actual library elsewhere, and have this repo simply depend on matching versions of it. Downside of that is we lose connection to all the current issues and PRs though, but it would certainly put this project on a more secure footing than it is now. @Seldaek - you should have the same access rights as I do on this repo, so if you are wondering about something, that will hopefully clarify what can be done without @justinrainbow's involvement, and what we would need to escalate his way. On the versioning thing - in my opinion, v6 should be released with PHP >=5.6 compat, and then immediately deprecated in favour of v7, which will require PHP >=7.2. That will keep the folks who use the master branch on very old projects (for draft-06 support) happy, as they'll have an actual version to link against, and we won't risk breaking stuff that doesn't pin a specific commit. At the same time, that will mean we can finally drop the 5.x requirement from master, which causes significant headaches. |
Thanks, good to know that, tho I am not sure where we find this person ;) But hopefully if @DannyvdSluijs is ready to do this spec work and others can review PRs from a purely code standpoint that'd already be progress.
I think that's a bad idea tbh. It'd be better to create an org.. not sure what name, best I could come up with that is not taken yet is
Sounds good. Is that something I could take care of right now? Does anything speak against just tagging current master? I mean if there are bugs these can be fixed later in patch releases.. But is there any breaking change that would still be required before calling it "done"/good enough? I'm happy to do the grunt work of release and new branches etc.. but I lack in-depth project knowledge to make that call. The other big blocker for me would be the changelog.. Aside from "draft-06 support" I'm not sure if there is anything notable for end users. Possibly, but the commit list is quite long. |
+1 for moving to an org sooner rather than later. If the current owner can transfer the repo, great, otherwise since there is commit access here we could either deprecate and suggest or even do the dependency-of-dependency move suggested a bit further up. I imagine larger projects like Composer and Drupal can point at a new project name, if necessary, easy enough and we can provide a smooth transition for the others. Easier to rip off that bandaid earlier while there is momentum than later when you need to under more pressure. |
jsonrainbow? |
This was my key concern. If it's a non-issue, then I agree that the switcheroo described is unnecessary, and we can just go ahead and move it. I don't particularly care what the org name is 🙂
I think yes.
No, I think we can call it good enough. Worth putting a note on the release that there are a few areas of non-conformity / lack of polish, but there are no show-stoppers I'm aware of, and many people already use v6 in prod already; this would just give that existing usage a formal version to refer to.
The big items are draft-06 support and changes to the way error handling works. Everything else is either comparatively minor, or has already been backported into the 5.x.x branch. |
Alright I created a jsonrainbow org (thanks @dafeder;). I'll reach out to Justin to try and get the migration done. |
Thanks @Seldaek 🙂 While you're talking with him, it might be a good idea to also ask him for control over the packagist side too. IMO it would make the most sense for you to be the owner of that. |
Yeah that does not worry me as much as I have full control there if something really needs to happen, but that's typically nothing as it just pulls from github.. |
It's quite inspiring to watch this all come together in real time. I'd like to contribute, I'm still just very unfamiliar with the actual code. In particular supporting more recent draft specs would be a priority for me (it's the main reason we use a mix of this and opis on https://github.com/getdkan/dkan). I want to be cautious about promising to do anything but I'm going to at least start getting caught up on what's been done up to this point and then basic code flow. I'll keep my eyes open for other ways to support as the backlog comes more into focus. |
I'm sure any help will be appreciated. When it comes to new drafts I would imagine just focusing efforts on supporting the latest draft and skipping everything between that and draft-06 is probably the most sensible? But I think we should first focus on getting v6 tagged, then ripping out legacy PHP versions and then it'll be a better base to build upon. |
Raising my hand over here... I'm on the same team as @dafeder and I can probably commit some time. |
@justinrainbow has just moved us over to the org. Thanks! |
❤️ Love the participationWow a lot has happened since my previous post, which shows there are people willing to help out of have already helped out. Thanks @erayd and @Seldaek for your efforts in the past few days and of course to @justinrainbow for the swift handover, thank you for the trust. ⏭️ Version 6.0.0I think getting 6.0 out the door is something doable before the first half of the year is over. As mentioned by others version 6 should keep the support of the current PHP version constraint (
These are both valid concerns and should be addressed, I guess a draft PR updating Composer to use the 🐎 Actionable itemsPerhaps @dafeder or @paul-m are willing to help out with the changelog for V6? If not I can prioritize to work on that. As a final note this issue is getting to active and covers multiple topics ATM, which is a luxury problem I know. Would it be a good idea to enable the discussions on this repo in order to discuss more in depth on different topics while also not polluting the issue list? @Seldaek is that something you could do? I dont have the permissions to make this happen. |
@DannyvdSluijs I enabled discussions and also added you as a a maintainer which should give you a bunch of permissions to manage the repo. For sure if someone can get me a changelog I'm happy to do the release, if not I'll get to it but it'll take longer probably :) |
@DannyvdSluijs yes could possibly help - can we decide on a new place to focus 6.0 release coordination? Either in discussions or #527 maybe? |
I've just started Version 6.0.0 release in Discussions |
As can be read in jsonrainbow#658, the repository has been moved. Update the composer.json homepage instruction to point to the new location.
Since the migration it drove me a bit nuts seeing this dummy gravatar icon on the repo so I took 5min to make a logo.. It's not amazing, but it's something :) Original files in a few sizes and PSD sources are here just in case anyone wants/needs it: JsonSchemaLogo.zip |
Love it! |
* Fix: Composer.json homepage As can be read in #658, the repository has been moved. Update the composer.json homepage instruction to point to the new location. * docs: Update remaining references to jsonrainbow --------- Co-authored-by: Floris Luiten <floris.luiten@tweakers.net>
I think we can close this issue for now? We've got the ball rolling, just need to keep momentum going now :) @paul-m @dafeder and anyone else interested in helping, I'd say the best is to watch the repo so you get alerts for new issues/PRs/.. Then for my part I'd be happy to grant more rights to anyone that consistently helps out, but even without that I'm sure any time spent reviewing PRs or closing issues is appreciated by everyone involved here. |
Unfortunately, I no longer have the time available to do this project justice, and have not for some time now. I've been dealing with things as I have the capacity to address them, but the PRs are coming in faster than I have time for, especially recently. I'm also the only active maintainer left, which is resulting in some pretty long delays in resolving things. This isn't good, given our significant downstream dependents.
I think we need to add another maintainer to take over the day-to-day stuff (PR reviews, merges, backports etc). If you are reading this and are keen to be involved in such a capacity, or would like to suggest a candidate, please comment on this issue.
@bighappyface - do you still have a comms channel with @justinrainbow? We will need him to deal with any new people added to the repo, as neither of us (nor @shmax, who has now moved on entirely) have the necessary admin permissions. If possible, it would be really good to get another person with admin permissions on the project (maybe me for now?) to reduce the bus factor here.
Also important that we be fairly careful about who we bring on board - we need to take into account the security implications if this project were to be compromised.
/cc @Seldaek - would appreciate your input on this; you're the most critical dependent we have.
The text was updated successfully, but these errors were encountered: