Switch from Pocketbase data retrieval to JSON #100
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…dpoint to aggregate data. Adjust type definitions for Script and Category
…amline interface/port handling for better clarity
…cript` types, simplifying the sorting logic and enhancing clarity
…ion functionality across the application
…s` to clean up code
…ilize centralized `fetchCategories` for improved maintainability
…stent badge rendering across script types
…ity and streamline the installation script logic
…d handle undefined values more gracefully in rendering
…d update copy button logic for improved functionality
… `ScriptInfoBlocks` component
…de script name for better clarity in the UI
… `DefaultSettings` component
Contributor
|
That was quick work! Well done 🚀 I wasn't actually aware you could call URLs like: https://api.github.com/repos/community-scripts/ProxmoxVE/contents/json I thought we would need to dynamically compile all the JSON into a single script on deploy, but this avoids that. Amazing! How resilient is the code if someone accidentally merges malformed json into a file inside /json? Will the build just fail? I have two suggestions before merging:
|
|
I think a json schema and an action that validates future PRs is a goal to have.Sent from my iPhoneOn Nov 6, 2024, at 1:42 PM, Paul ***@***.***> wrote:
That was quick work! Well done 🚀
I wasn't actually aware you could call URLs like: https://api.github.com/repos/community-scripts/ProxmoxVE/contents/json
I thought we would need to dynamically compile all the JSON into a single script on deploy, but this avoids that. Amazing!
How resilient is the code if someone accidentally merges malformed json into a file inside /json? Will the build just fail?
I have two suggestions before merging:
Remove the following pocketbase related files:
/frontend/src/lib/pocketbase.ts
/frontend/example.env
/frontend/.env.local
There is also a number of occurrences of the string "https://community-scripts.github.io/Proxmox/" that should be replaced with "https://community-scripts.github.io/Proxmox/VM", or put inside an environment variable rather than being hardcoded.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
Member
Author
i dont actually know. i think wont fail but it will cause problems. i can start work on a JSON validator that people can use to check if everything is valid. i can then also make it so that if a script is not valid. that it will automatically excluded. |
…Ls for improved adaptability and host configuration
…tics and base path; replace PocketBase imports with new data module
…enhancing adaptability for URL management
Co-authored-by: Håvard Gjøby Thom <34199185+havardthom@users.noreply.github.com>
…gs.tsx Co-authored-by: Håvard Gjøby Thom <34199185+havardthom@users.noreply.github.com>
…gs.tsx with new settings configuration
… adjustments and improvements
Member
Author
|
@havardthom everything should be applied now. thank you! |
…flect changes in siteConfig
havardthom
reviewed
Nov 6, 2024
havardthom
reviewed
Nov 6, 2024
frontend/src/app/scripts/_components/ScriptItems/DefaultSettings.tsx
Outdated
Show resolved
Hide resolved
havardthom
reviewed
Nov 6, 2024
frontend/src/app/scripts/_components/ScriptItems/DefaultSettings.tsx
Outdated
Show resolved
Hide resolved
havardthom
reviewed
Nov 6, 2024
Co-authored-by: Håvard Gjøby Thom <34199185+havardthom@users.noreply.github.com>
Co-authored-by: Håvard Gjøby Thom <34199185+havardthom@users.noreply.github.com>
Co-authored-by: Håvard Gjøby Thom <34199185+havardthom@users.noreply.github.com>
… type in map function
Contributor
|
Looks like some basepath error on https://bramsuurdje.github.io/ProxmoxVE/scripts (request to |
…rn type in fetchScripts map function
…ove array concatenation method for better readability
… correct path structure in API routes
Member
Author
|
Should be good to go now. |
Member
|
@havardthom Please merge on your own if you think it's okay. :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note
We are meticulous when it comes to merging code into the main branch, so please understand that we may reject pull requests that do not meet the project's standards. It's never personal. Also, game-related scripts have a lower chance of being merged.
Description
In this pull request. the data fetching of the website will change from the old Pocketbase system to the new JSON based system. making it easier to maintain in the future.
Type of change
Please check the relevant option(s):