Skip to content

Conversation

@psrpinto
Copy link
Member

@psrpinto psrpinto commented Dec 10, 2024

This PR merges the schemas/ directory (#145) into schema/ (from #146).

Additionally, it makes some extra changes:

  • Because when we do schema validation we have to load all schemas into memory, I thought it would make sense that we also generate the final schema.json at that point. So here I changed validate.mjs into build.mjs which now does both validation and generation of the final schema.json.
  • The final schema.json file is generated into the schema/ directory itself, but hidden from git
  • Then I changed webpack so that it just calls build.mjs and then reads the generated schema.json, instead of webpack itself having logic for generating the final schema.json.
  • This makes it possible to centralize the logic of building the schema into one place (schema/build.mjs), which can also be ran from the CLI.
  • I also moved the individual schemas into a schema/subjects directory, so that they are separate from all other files
  • I also added a build:schema script to package.json
Screenshot 2024-12-10 at 17 21 27

@psrpinto psrpinto marked this pull request as ready for review December 10, 2024 17:23
@psrpinto psrpinto requested review from akirk and ashfame December 10, 2024 17:23
@psrpinto psrpinto added this to the MVP milestone Dec 10, 2024
@psrpinto psrpinto self-assigned this Dec 10, 2024
@psrpinto
Copy link
Member Author

psrpinto commented Dec 10, 2024

There's currently an issue that causes the npm run start:firefox to go into an infinite loop. That issue is already present on trunk, so it's not introduced by this PR. I'm looking into it.

@ashfame
Copy link
Member

ashfame commented Dec 11, 2024

Fixed in #150

@psrpinto
Copy link
Member Author

Cool, thanks for fixing. Should we merge this one? Or you wanted to merge #150 into here?

@ashfame
Copy link
Member

ashfame commented Dec 11, 2024

@psrpinto Will merge #150 in this one and then you can merge it to trunk, if that's alright?

@psrpinto
Copy link
Member Author

Yep, that works 👍

Copy link
Member

@ashfame ashfame left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done!

@psrpinto psrpinto requested a review from ashfame December 11, 2024 15:42
Copy link
Member

@ashfame ashfame left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@psrpinto psrpinto merged commit 85344d2 into trunk Dec 11, 2024
3 checks passed
@psrpinto psrpinto deleted the schema-files-build branch December 11, 2024 15:45
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