Fetch and checkout pr branch on gitpod if LILA_PR variable is provided and get preview easily #60
Conversation
I believe you can enable them for your fork if you visit https://github.com/Carbrex/lila-docker/actions as the repo owner Looks like the docker build is failing because of an issue with the mobile repo. Could be temporary or maybe the dependencies or setup changed. I'll investigate. |
|
Testing it out now. This is great! Feel free to push as many followup commits as you want. I'll squash merge once it's ready. |
There was a problem hiding this comment.
I think when a PR is passed, we can skip the other prompts (optional services, passwords, etc) and go for an opinionated setup so that things just boot right away. In the future, we could control the optional services using the same technique if they're needed. Doesn't have to be a part of this PR, but I might try to do that if you don't want to.
So, do I not add any optional services, when the lila_pr variable is passed? Thanks for fixing the failed tests thing |
|
Should Cargo.lock be in .gitignore? EDIT: nvm, I think it should not be in .gitignore |
|
main.rs is now too big to be in a single file? |
Yes, you can commit it. *EDIT: my mistake, I meant commit the lock file
I think it's OK. We'll ask @kraktus for a Rust review once it's ready, to be sure. |
|
Looks like the functionality broke after these changes. |
|
Thanks for fixing it. I got it working on my patch-3 branch though. https://github.com/Carbrex/lila-docker/tree/patch-3 A separate lila_pr variable would help in making the other changes you mentioned above?
|
|
Yep good point, how about we add both to the struct. keep Didn't mean to step on your toes with the changes. I had some in-progress edits while reviewing when you had pushed. I'll pause on making any changes until you've finalized. |
Ok, will complete within 10 minutes and then you can push your changes. Thanks for helping. |
|
I have done my changes in patch-3 but I am having trouble merging my changes with yours. https://github.com/Carbrex/lila-docker/pull/1/files |
* Trying to add lila PR checkout functionality in setup() function * Try fixing expected `Result<(), Error>`, found `()` error * Remove semicolon to fix another return error * Remove return Ok(); * Move remaining part to else in main.rs setup * Add environment variable printing and handle missing PR number * Refactor setup function to handle missing environment variable GITPOD_GIT_PR_NUMBER * Get Environment variable from nested Gitpod_workspace_context * Add serde_json to dependencies * Try fixing match error * Add print statements to check where the functionality broke * Update workspace context print statement * Update lila_pr_no type to String * Remove debug print statements in main.rs * Refactor load_lila_pr_no function * Add return * Merge patch-4 to patch-3 * Add gitpod_checkout_pr function to load and checkout a specific pull request * Merge changes * Maybe fix stack overflow? * Refactor loading LILA PR number in Gitpod struct * Remove load_lila_pr fn * Clipy fix * Fix formatting * Fix more formatting
|
Finally done. You can proceed now. Sorry for all this clutter. |
|
tl;dr - Optional URL key/value will start a Gitpod workspace with a PR checked out instead of master. Example: https://gitpod.io/new/#LILA_PR=14738/https://github.com/Carbrex/lila-docker/tree/patch-4 Those key/value pairs get automatically added to GITPOD_WORKSPACE_CONTEXT env so this PR parses and then runs the necessary git commands. |
|
I’ll check tomorrow 👍 |
These packages are stable and follow semvers.
|
Nice work, only changed a few nits. The failure of test is not due to change, the tests are inherently flickered because of setting env variables for their own process. |
Closes #17
Try this
https://gitpod.io/new/#LILA_PR=14733/https://github.com/Carbrex/lila-docker/tree/patch-4
Works well on my local and also on gitpod. Any idea why the docker compose build test failed.
Also why doesn't these tests show up on my fork like how the lila's do.