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

split manifest and use asyncio #319

Merged
merged 5 commits into from
Jul 6, 2024
Merged

split manifest and use asyncio #319

merged 5 commits into from
Jul 6, 2024

Conversation

taooceros
Copy link
Member

@taooceros taooceros commented Feb 4, 2024

Script to split is simple

let plugins = open "plugins.json"

for plugin in $plugins {
    let name = $plugin.Name
    let id = $plugin.ID

    $plugin | to json -i 4 | save $"plugins\\($name)-($id).json" -f
}

@taooceros
Copy link
Member Author

@jjw24 want to check?

@jjw24
Copy link
Member

jjw24 commented Feb 13, 2024

What's the performance gain from this change?

@jjw24
Copy link
Member

jjw24 commented Feb 13, 2024

And what have we tested with these changes?

If deleting the plugin file, will it remove it from plugin.json?

@jjw24
Copy link
Member

jjw24 commented Feb 13, 2024

Would it be better to just allow dropping the plugin's json file into the plugin folder and then the script picks out the fields to form the plugins.json and test + validate the plugins?

@taooceros
Copy link
Member Author

Would it be better to just allow dropping the plugin's json file into the plugin folder and then the script picks out the fields to form the plugins.json and test + validate the plugins?

That's almost the case. Instead we just test and validate before forming the plugins.json

@taooceros
Copy link
Member Author

What's the performance gain from this change?

Almost proportional to the number of plugins

@taooceros
Copy link
Member Author

And what have we tested with these changes?

If deleting the plugin file, will it remove it from plugin.json?

CI should periodically generate plugins.json from the directory

@Garulf
Copy link
Member

Garulf commented Feb 14, 2024

Hmm if the plugin.json is merged from all individual plugin entries should we create a release for the plugins.json?

Instead of committing it to the repository.

@jjw24
Copy link
Member

jjw24 commented Feb 14, 2024

Are we sure we are not just complicating things unnecessarily?

@jjw24
Copy link
Member

jjw24 commented Feb 14, 2024

Because instead of editing a single json file now will have to create a seperate file, which is a different process as well. I'm sure editing json isn't that difficult, and the CI isnt running for too long.

@taooceros
Copy link
Member Author

Are we sure we are not just complicating things unnecessarily?

I wouldn't say editing a huge file is less complex than adding a new file specifically. Almost all plugin manifests uses a dedicated file/folder for each plugin.

The merge conflict is really complicating things and annoying.

@taooceros
Copy link
Member Author

Are we sure we are not just complicating things unnecessarily?

I am not sure. Maybe that's good? Though what's the difference.

@jjw24
Copy link
Member

jjw24 commented Feb 16, 2024

Would it be better to just allow dropping the plugin's json file into the plugin folder and then the script picks out the fields to form the plugins.json and test + validate the plugins?

That's almost the case. Instead we just test and validate before forming the plugins.json

So would the test validation fail if the file has extra fields like "Actionkeyword"? If doesn't fail will plugin.json still form properly with only the required fields?

@taooceros
Copy link
Member Author

We still need extra information. Like the repo position and the icon. But yes I agree it's not a big difference.

@taooceros
Copy link
Member Author

Would it be better to just allow dropping the plugin's json file into the plugin folder and then the script picks out the fields to form the plugins.json and test + validate the plugins?

That's almost the case. Instead we just test and validate before forming the plugins.json

So would the test validation fail if the file has extra fields like "Actionkeyword"? If doesn't fail will plugin.json still form properly with only the required fields?

I use the same ci. So probably not.

@VictoriousRaptor
Copy link
Contributor

Are we ready to push this a bit further?

@jjw24
Copy link
Member

jjw24 commented Jun 25, 2024

Sure, let's get the latest plugin files in and I will take another look.

@taooceros taooceros enabled auto-merge (squash) June 29, 2024 18:36
@taooceros
Copy link
Member Author

taooceros commented Jun 29, 2024

@jjw24 done

@taooceros taooceros merged commit 6d5eb31 into plugin_api_v2 Jul 6, 2024
7 checks passed
@taooceros taooceros deleted the split-manifest branch July 6, 2024 09:38
@jjw24 jjw24 added the enhancement New feature or request label Jul 6, 2024
jjw24 added a commit that referenced this pull request Jul 6, 2024
Remove requests module import. From #319
This was referenced Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants