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 Compatibility for category and categories in Properties Files #45

Open
SableRaf opened this issue Jan 11, 2025 · 5 comments
Open

Add Compatibility for category and categories in Properties Files #45

SableRaf opened this issue Jan 11, 2025 · 5 comments

Comments

@SableRaf
Copy link
Contributor

SableRaf commented Jan 11, 2025

Continuing the conversation with @mingness started in #42

Do you want to allow a library to submit without a category? If so, then the validate script needs updating, wrt optional or required fields.

Thanks for catching that! The properties file for LazyGUI (which is the library I picked as an example for testing my changes to the issue templates) is using category instead of categories which caused the workflow to error out.

The original script appears to have gracefully handled this by mapping category to categories when creating the contribs file.

To maintain compatibility with both formats (category and categories), we should update the script to support this mapping.

Regarding the case where category or categories are both missing or unpopulated, it looks like the original script set categories as null if it couldn't find it though I don't know if that was intentional (see here for example). I'd lean towards throwing an error and asking the contributor to pick at least one category.

@mingness
Copy link
Collaborator

mingness commented Jan 11, 2025

The code already allows for existing properties files that use category

categories: Optional[str] = Field(None, alias='category')

The code just doesn't allow for new properties files to use category, instead forcing the use of categories.
categories: Optional[str] = Field(None)

Would you like to change this? Basically, a properties file like LazyGUI's, should it or should it not be allowed to be submitted as a new library properties file? The way the code is now, I'm loose when reading in files for updating the database, but strict when accepting new ones in submissions.

For completeness, categories are required for libraries

@mingness
Copy link
Collaborator

I'm using Pydantic for data validation - I had manually tested for all these cases before it started getting unwieldy.

@SableRaf
Copy link
Contributor Author

Would you like to change this? Basically, a properties file like LazyGUI's, should it or should it not be allowed to be submitted as a new library properties file?

I think writing category instead of categories is an easy mistake to make, especially if you are making a library from scratch. Is there a drawback to keeping the category > categories mapping?

@mingness
Copy link
Collaborator

It's an easy mistake, but it's a mistake that only has to be corrected once, and then it's not an issue. I see it as a consistency and simplicity thing. Why accept category and not author in that case? Technically, Pydantic makes it easy to accept aliases, but I have no idea how to code this in kotlin for instance - if we want to eventually move the scripts to Kotlin, do they have something like the Pydantic library?

@SableRaf
Copy link
Contributor Author

I think the fact that both recent submissions used category shows it feels more natural for contributors. I'd say we should support category, author and any other singular/plural aliases for the same reason. It’s a small adjustment on our side that could reduce friction. What do you think?

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

No branches or pull requests

2 participants