hyperupcall marked 2 inline comments as done. hyperupcall added a comment. In D140462#4018983 <https://reviews.llvm.org/D140462#4018983>, @sammccall wrote:
> The difficulty in testing is that: > > - JSON-schema is totally unknown in llvm-project, so there are no tools but > also no expectation that contributors understand it > - llvm-project doesn't like dependencies, particularly those that aren't in > C++ or python > - having done some digging, interop seems really poor... > > I tried 3 consumers (`ajv-cli`, `yaml-language-server`, and `yajsv`) and > **hit different blocking bugs in all of them** when using obvious, > spec-compliant approaches to writing schemas. I only have experience with > `ajv`, so I suppose it means > It has the "python problem", where it's readable and you can see at a glance > why the code is correct - even if it isn't. > (Having spent a couple of days with json-schema now, I **don't** think it's > straightforward. That file seems simple so far, but there are simple C++ > programs too! And it carefully dodges the interop issues, which is *subtle*). I appreciate that you took the time to experiment with those three implementations of JSON-schema - perhaps I was naive in thinking that the different implementations were more mature. On my side, this makes me want to validate the various schemas in SchemaStore with more implementations than just `ajv` to improve the experience for everyone. > --- > > Fixing this is an unreasonable burden to place on a new contributor, I've > taken a shot at this and have something almost working: > https://reviews.llvm.org/D140745. > > It's pretty draft-ish: > > - That patch just checks in the generated files {`Fragment.inc`, `YAML.inc`, > `doc.md`, `schema.json`}, (maybe) that belongs in the build system > - the schema is correct but `yaml-language-server` chokes on it due to > https://github.com/redhat-developer/yaml-language-server/issues/823, so need > to restructure > - files would need to be moved around etc > - tests need to be added/updated > - i'm not sure whether having a json-schema for `schema.yaml` is actually a > good idea, it was mostly a learning exercise This looks pretty neat! I think something like that revision would be a better way forward. I will go ahead and upload a new Diff, I suppose just for completeness, but I don't expect anything to change much since it doesn't address your points about syncing with documentation or the obscurity of the specification. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140462/new/ https://reviews.llvm.org/D140462 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
