Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

added gitpod config #10

Closed
wants to merge 1 commit into from
Closed

added gitpod config #10

wants to merge 1 commit into from

Conversation

svenefftinge
Copy link
Member

@svenefftinge svenefftinge commented Nov 1, 2021

This PR adds a config so that users can run the installer from within gitpod.
It also fixes #6 and runs some commands (e.g. creating and opening .env) to streamline the process.

@svenefftinge svenefftinge marked this pull request as ready for review November 1, 2021 20:44
Copy link
Member

@aledbf aledbf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

mkdir -p ~/.config/gcloud
open .env
gcloud auth login
make install
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you run make install at the end of this won't it try to just launch the installer with the example values? That is before this user has had the chance to modify to suit their account. Perhaps a make help so they see what the options are plus a "edit .env and run make install when you are ready"

That confused me for a minute before I realized I didn't do anything wrong.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, I opened the workspace saw the Google oauth prompt right away, connected that, before I edited the env. Knee jerk reaction to being prompted for a login, "do it quickly or something will timeout". Then I wasn't sure how to restart. Had to dig into the gitpod to understand get to oh just re-run make install.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree this can be further streamlined. Maybe the install should initially check for the required env vars (BILLING_ACCOUNT and DOMAIN) and fail early if they haven't been set.
That said I'm still not sure what to do about the DOMAIN at all 😁

@mrsimonemms
Copy link
Contributor

For visibility, I'd like to get the rest of the work required on #12 finished off before we look at this. It may be that much of the Installer stuff fixes these issues and, in any case, there's likely to be conflicts between the two

@svenefftinge
Copy link
Member Author

Thanks for the heads up. Any eta on when the PR lands?

@mrsimonemms
Copy link
Contributor

@svenefftinge I've set it as my team target to complete it this week

@mrsimonemms
Copy link
Contributor

I'm going to close this as we'll be doing a complete overhaul of the guides in the new year

@ionutale
Copy link

I'm going to close this as we'll be doing a complete overhaul of the guides in the new year

@mrsimonemms could you add some context, please?

@mrsimonemms
Copy link
Contributor

@ionutale we're moving to a monthly release cycle before the end of Jan. The docs will be released at the same time

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If pre-emptible=false, make install fails
5 participants