- 
                Notifications
    You must be signed in to change notification settings 
- Fork 19
add new snowflake samples page #295
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
base: main
Are you sure you want to change the base?
Conversation
| Deploying localstack-docs with   | 
| Latest commit: | f2f5742 | 
| Status: | ✅ Deploy successful! | 
| Preview URL: | https://1120792d.localstack-docs.pages.dev | 
| Branch Preview URL: | https://snowflake-sample-apps-page.localstack-docs.pages.dev | 
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.
LGTM - left a minor comment.
I skimmed all astro-specific files, assuming they will be sound after your own careful checking and review from docs team.
Would like @hovaesco to double check the snowflake sample app metadata is correct if he has time e.g. in src/data/developerhub/applications.json
| "helm-charts": "Helm Charts", | ||
| "pulumi": "Pulumi" | ||
| "pulumi": "Pulumi", | ||
| "snowcli": "Snow CLI" | 
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.
it's actually called Snowflake CLI (https://docs.snowflake.com/en/developer-guide/snowflake-cli/index)
confusingly there's a related older tool called SnowSQL (https://docs.snowflake.com/en/user-guide/snowsql)
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.
LGTM! Do we want to include multi-container app as well https://github.com/localstack-samples/localstack-snowflake-samples/tree/main/multi-container ?
| collapsed: true, | ||
| autogenerate: { directory: '/snowflake/integrations' }, | ||
| }, | ||
| { | 
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.
do we want to maintain the same page order in the navigation panel as in AWS docs?
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.
I think it makes sense, although not critical. It gives our docs a consistent feel.
| do you want us to merge it? @HarshCasper 😄 | 
| 
 Yes. unless more work needs to be done for multi-container app to include it. If that's the case I'd raise another ticket for it. @HarshCasper can we include the multi-container app as is? | 
| 
 Doesn't seem like Harsh had a chance to look at the review comments yet? @HarshCasper | 

No description provided.