-
Notifications
You must be signed in to change notification settings - Fork 3
Phani/GitHub deploy docs #80
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
Conversation
Mesa DescriptionAdd GitHub deployment documentationAdded comprehensive documentation for deploying Kernel apps directly from GitHub repositories, covering:
The deployment guide has been reorganized with clearer hierarchy, presenting "From local directory" and "From GitHub" as sibling deployment methods under "Deploy the app" for improved developer experience and discoverability. Description generated by Mesa. Update settings |
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.
Performed full review of e68644d...946865c
Analysis
While this PR enhances GitHub-based deployment functionality, there are no explicit architectural issues identified in the analysis. The analysis only highlights positive aspects such as good abstraction, feature discoverability, information architecture, and API design. Without additional critical feedback, I cannot identify specific architectural concerns with the implementation.
Tip
⚡ Quick Actions
This review was generated by Mesa.
Actions:
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
2 files reviewed | 0 comments | Review on Mesa | Edit Reviewer Settings
masnwilliams
left a comment
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.
Just a few things here
|
|
||
| ## Environment variables | ||
|
|
||
| You can set environment variables for your app using the `--env` flag. For example: |
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.
Can we also show an example using --env-file .env with an env file instead of in-line values?
I find this is an easier pattern to replicate and one I use most consistently. I know this isn't exactly related to the branch, but thought I would just ask to have it included.
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.
Added! Now showing both --env inline and --env-file .env examples in the CodeGroup.
| ``` | ||
|
|
||
| #### Notes | ||
| - **`--path` vs `--entrypoint`:** Use `--path` to specify a subdirectory within the repo (useful for monorepos), and `--entrypoint` for the path to your app's entry file relative to that directory (or repo root if no `--path` is specified). |
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.
Does this apply to local directory deployment too? If so, I would suggest moving it to the Deployment Notes section that is more general.
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.
The local directory deployment only uses entrypoint_file_name. It is specific to the github approach only.
| - **The entrypoint file and dependency manifest (`package.json` for JS/TS, `pyproject.toml` for Python) must both be in the root directory of your project.** | ||
| - Include a `.gitignore` file to exclude dependency folders like `node_modules` and `.venv`. | ||
| - **The dependency manifest (`package.json` for JS/TS, `pyproject.toml` for Python) must be present in the root directory of your project.** | ||
| - View deployment logs using: `kernel deploy logs <deployment_id> --follow` |
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.
Can we add a bullet here that says something like "If you see 500 error returned during deployment, please ensure you have specified the entrypoint file name and type correctly.". We had a ticket in Linear related to a user seeing this error.
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.
Yeah, that's a nice to have!
Something like this should be good:-
- If you encounter a 500 error during deployment, verify that your entrypoint file name and extension are correct (e.g.,
app.pynotapporapp.js).
lmk what you think?
masnwilliams
left a comment
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
Add GitHub deployment documentation
Added comprehensive documentation for deploying Kernel apps directly from GitHub repositories, covering:
--url,--ref,--entrypoint,--path,--github-token, and environment variable configurationThe deployment guide has been reorganized with clearer hierarchy, presenting "From local directory" and "From GitHub" as sibling deployment methods under "Deploy the app" for improved developer experience and discoverability.