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 ZModel language #7065

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

jiashengguo
Copy link

@jiashengguo jiashengguo commented Sep 28, 2024

Description

Checklist:

@jiashengguo jiashengguo requested a review from a team as a code owner September 28, 2024 03:58
@lildude
Copy link
Member

lildude commented Sep 28, 2024

We don't add frameworks, stacks, libraries or toolkits as their own languages. This project identifies itself as a "Fullstack TypeScript toolkit that enhances Prisma ORM with flexible Authorization layer for RBAC/ABAC/PBAC/ReBAC, offering auto-generated type-safe APIs and frontend hooks." which appears to be entirely Typescript.

React isn't its own language in Linguistic for this same reason. Closing.

@lildude lildude closed this Sep 28, 2024
@jiashengguo
Copy link
Author

jiashengguo commented Sep 28, 2024

@lildude
Thanks for the explanation. I see the point, but I'd like to make some clarifications.

  1. ZenStack is indeed a TypeScript toolkit, but the language I'd like to add is the ZModel language it uses. ZModel is a domain-specific language (DSL) for defining data models and access policies, which is actually language-agnostic.
    ZModel Language

  2. ZModel is a superset of the Prisma Schema Language (.prisma). Given that Prisma schema is already included in Linguistic, I believe ZModel should also qualify for inclusion.

  3. We once positioned ZenStack to be a "DSL for defining data and access rules closer to the database". You can see our Hacker News launch long time ago:
    https://news.ycombinator.com/item?id=37750972
    We don’t mind changing our GitHub description to so if it would make some difference.

  4. To be able to get the syntax highlight is actually repuested by some ZenStack’s users. Below is the issue created by one of them:
    Provide grammar to Linguist project #823

Since there are already hundreds of users using it, I definitely hope they could get a better DX on GitHub. If you still hold the opinion, may I know what’s the prerequisites it must meet to be qualified for ZModel to get in?

@lildude
Copy link
Member

lildude commented Sep 28, 2024

Thanks for the explanation. I’ve reopened it but you need to rename the language to Zmodel as that is what you are adding. Popularity isn’t sufficient for merging now anyway.

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

The language you are adding is zmodel, not Zenstack. Please rename.

@jiashengguo jiashengguo changed the title Add ZenStack language support Add ZModel language support Sep 28, 2024
@jiashengguo
Copy link
Author

Thanks for the explanation. I’ve reopened it but you need to rename the language to Zmodel as that is what you are adding. Popularity isn’t sufficient for merging now anyway.

Hey @lildude , may I know what the up-to-date polarity assessment criteria is?

According to the assessment mentioned in #5756

  • At least 2000 files per extension were indexed in the last year (the number you see at the top of the search results) unless the extension is expected to occur only once per repo, then 200 files.
  • with a reasonable distribution across unique :user/:repo combinations assessed by manually and randomly clicking through the results.

For Zmodel, the extension is expected to occur only once per repo. Using the search offered in the template, the result is 387 which is more than 200.
https://github.com/search?type=code&q=NOT+is%3Afork+path%3A*.zmodel+model

So, I believe the first rule has been met. Could the issue be with the second rule? If so, is there a specific way I can verify this myself?

@lildude
Copy link
Member

lildude commented Sep 29, 2024

For Zmodel, the extension is expected to occur only once per repo. Using the search offered in the template, the result is 387 which is more than 200.

🤔 that doesn't appear to be the case from a quick look through the search results like this repo, or any of the other results where the .zmodel file returned is not in the root of the repo. Your own project has more than one.

This suggests the files will occur at least once per repo and likely more, hence the first criteria applies.

@lildude
Copy link
Member

lildude commented Sep 29, 2024

Of course this all makes sense when you consider Prisma models will be for many DB tables and people prefer to break things up for easier maintenance (and ownership in big monorepos) as is the case I first referenced which is taking full advantage of importing the individual .vmodel files in their schema.vmodel.

So I'd say schema.vmodel is likely to occur only once per repo, but not the extension by the design of the fact the individual files can be imported as needed.

@lildude lildude changed the title Add ZModel language support Add ZModel language Sep 29, 2024
Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

You will also need to rename the directory containing the samples.

lib/linguist/languages.yml Outdated Show resolved Hide resolved
lib/linguist/languages.yml Outdated Show resolved Hide resolved
@jiashengguo
Copy link
Author

jiashengguo commented Sep 29, 2024

@lildude Thanks for the comments and the detailed explanation. The comments has been resovled.

You're right; it is the same case as Prisma, where typically users have only one schema file as their database model in the repo.

The reason we supported multiple schema files because that was a long-requested feature by the user of Prisma when their schema grows to a certain level for better maintenance (Prisma also added preview support recently after we supported it):

Support for splitting Prisma schema into multiple files #2377

Even if people use multiple schema files, there would be a main entry file that could only occur once per repo. So you can use the below search to see the unique repos that use zmodel:

https://github.com/search?type=code&q=NOT+is%3Afork+path%3A*.zmodel+datasource

@jiashengguo jiashengguo requested a review from lildude October 5, 2024 05:04
@jiashengguo
Copy link
Author

@lildude The changes have been made based on your feedback. Is there anything else I need to do?

As I mentioned above, since most repos typically have just one schema file unless they break things up for easier maintenance (same for Prisma), I wonder if it might meet the popularity criteria now. Here is the search for unique repos that use zmodel:

https://github.com/search?type=code&q=NOT+is%3Afork+path%3A*.zmodel+datasource

@ymc9
Copy link

ymc9 commented Nov 19, 2024

Hello @lildude , any chance we can get this merged soon? Thank you!

@lildude
Copy link
Member

lildude commented Nov 19, 2024

I don't review PRs for popularity requirements etc after the initial review until I'm close to making the next release. The next release will be made sometime next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants