Skip to content
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

add ksy_schema as submodule #1101

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

Conversation

rotu
Copy link

@rotu rotu commented Apr 3, 2024

No description provided.

@GreyCat
Copy link
Member

GreyCat commented Apr 4, 2024

@rotu, can you elaborate what is the objective we're trying to achieve here?

E.g. if we plan to release that schema as part of compiler installation, it will be probably better to include it into compiler rather than into the full build?

@generalmimon
Copy link
Member

I'm also not sure about this change. I don't think every kaitai-io repo needs to be a submodule of kaitai_struct.

Maybe it's somewhat good for visibility to add ksy_schema (doc is already here, and ksy_schema is not that far from that, but doc is arguably much more important and core repo of the project than ksy_schema), but a bit weird.

@rotu
Copy link
Author

rotu commented Apr 4, 2024

Ya. My objective here is solely DX. When searching the codebase for a keyword, I want to search the schema as well which is easier if they have a common root.

That said, I don’t get the benefit of splitting the project across so many repos. For example, it precludes implementing a new compiler feature, adding it to the schema and creating tests in a common PR.

@rotu
Copy link
Author

rotu commented Apr 4, 2024

BTW, kaitai_struct_doc already depends on the schema, where it is not explicitly versioned.

I do think of the schema as a sort of "executable documentation", but I don't quite know how to correctly keep it in sync with both the compiler and the docs.

Suggestions welcome!

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