-
-
Notifications
You must be signed in to change notification settings - Fork 130
Add snowshare #1063
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 snowshare #1063
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beofre i review that further:
You missed some coding standards we keep around here, but overall it dose not look to bad. First look in the latest merged Scripts for exampled. Second: We habe helper functions in tools.func to set up Node.js and Postgress.
3. We dont do git pulls, just downloads of releases. Ther also exists a helper function in tools.func.
Also please remove any comments, also you can then remove the jq check in the update part.
You can ping me when you did the changes, then i will have a deeper look into this pr.
Co-authored-by: Michel Roegl-Brunner <[email protected]>
Co-authored-by: Michel Roegl-Brunner <[email protected]>
install/snowshare-install.sh
Outdated
| msg_ok "Installed SnowShare dependencies" | ||
|
|
||
| msg_info "Creating Environment Configuration" | ||
| cat <<EOF >/opt/snowshare/.env |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to /opt/snowshare.env if possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this still applies (move to /opt/snowshare.env)
…into add-snowshare
Removed logging messages for database migrations and service creation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks already pretty good, just some smaller things
|
i think its ok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some cosmetic things then good to me
install/snowshare-install.sh
Outdated
| msg_ok "Installed SnowShare dependencies" | ||
|
|
||
| msg_info "Creating Environment Configuration" | ||
| cat <<EOF >/opt/snowshare/.env |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this still applies (move to /opt/snowshare.env)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TuroYT I fixed the env location for you.
Please check if build works
|
@CrazyWolf13 the build dont work with the snowshare.env when npx prisma migrate deploy : |
Scripts wich are clearly AI generated and not further revied by the Author of this PR (in terms of Coding Standards and Script Layout) may be closed without review.
✍️ Description
🔗 Related PR / Issue
Link: #
✅ Prerequisites (X in brackets)
🛠️ Type of Change (X in brackets)
README,AppName.md,CONTRIBUTING.md, or other docs.🔍 Code & Security Review (X in brackets)
Code_Audit.md&CONTRIBUTING.mdguidelinesAppName.sh,AppName-install.sh,AppName.json)📋 Additional Information (optional)