Skip to content
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

Upsun Quick start guide #3554

Merged
merged 79 commits into from
Feb 22, 2024
Merged

Upsun Quick start guide #3554

merged 79 commits into from
Feb 22, 2024

Conversation

flovntp
Copy link
Contributor

@flovntp flovntp commented Oct 31, 2023

I give a try to use the Express guide as the generic Quick start guide.

Feel free to change anything in that PR.

And i split every part of the Express (monolith) guide in different file for the main steps to appears in the left menu.

Why

Closes #{ISSUE_NUMBER}

What's changed

@gilzow
Copy link
Collaborator

gilzow commented Oct 31, 2023

also see comments in slack, re: making changes section

@gilzow
Copy link
Collaborator

gilzow commented Oct 31, 2023

another one: strapi out of the box doesn't working following the generic guide since it REQUIRES an environment variable APP_KEYS to be present, and the current upsun ify command doesn't include it.

The current Strapi guide includes adding it. The strapi-generated .env file also includes it but we don't parse/support .env files.

So, do we make Getting Started supplements for each stack that include these pieces that aren't included with upsun ify or do we keep pushing them into the upsun ify command?

Wait, it looks like you already mentioned it in your previous issue. I re-opened the issue and mentioned the APP_KEYS are missing.

sites/friday/src/start/here/setup.md Outdated Show resolved Hide resolved
sites/friday/src/start/here/setup.md Outdated Show resolved Hide resolved
sites/friday/src/start/here/setup.md Outdated Show resolved Hide resolved
@chadwcarlson
Copy link
Collaborator

gilzow
gilzow previously requested changes Feb 20, 2024

Then, you are asked if you want to set the local remote to your new project. Enter **Yes (y)**.

Your local source code is automatically linked to your newly created {{% vendor/name %}} project through the creation of a `.{{% vendor/cli %}}/local/project.yaml` file. This file contains the corresponding `<projectId>` and sets a Git remote to `{{% vendor/cli %}}`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this section isn't related to the note for "Default branches". It's also relevant to the console path after the user runs upsun project:set-remote. We should probably move this to the end of the page after the two sections in an area on its own.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i duplicate this section into the console path, is that ok?

sites/friday/src/get-started/here/create-project.md Outdated Show resolved Hide resolved
sites/friday/src/get-started/here/create-project.md Outdated Show resolved Hide resolved
upsun project:set-remote {{< variable "PROJECT_ID" >}}
```

Then push your code to Upsun with the command `upsun push`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The focus of this page is Create a project and has two paths for doing so. Pushing our code is relevant to both paths but is not related to Creating a project. I would suggest either making a new page "Your first push" OR add it to the Configure your project page as a first section where we can explain that the failure is due to not having configuration files, and lead them into how to configure the project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, i move the error on first push from both console and CLI path into the configuration page with a special section...
But i'm wondering if we should move this section into the troubleshoots section of the doc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

memcached (type: memcached:1.6, cpu: 0.5, memory: 1088)
```

{{% note theme="info" %}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We haven't discussed resource allocations at this point (beyond the mention of defaults), nor branching. IMO, this note should be moved to the here/get-started/set-resources.md file

Copy link
Collaborator

Choose a reason for hiding this comment

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

or possibly get-started/here/make-changes.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is already in a Next steps section, what is wrong here?

The new environment inherits the data (service data and assets) of its parent environment (the production environment here).
<--->
+++
title=Using third party provider
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we've removed the 3rd party integration sections in the earlier steps, and moved it to its own section later in the guide, we should consider removing this section here since 3rd party integrations haven't introduced at this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We didn't remove 3rd party integration so do i still need to move it to its own section?

sites/friday/src/get-started/here/make-changes.md Outdated Show resolved Hide resolved
```bash {location="Terminal"}
git branch checkout -b feat-a && git push -u upsun feat-a
```
**Secondly**, if you have set up a [source integration](/integrations/source/_index.md), make sure you disable the `prune_branches` option (`true` by default). Otherwise, the `upsun branch` command fails at the end of the deployment.
Copy link
Collaborator

Choose a reason for hiding this comment

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

if we keep the split path between Upsun Git repository and Using third party provider, we should move this second warning to the bottom of the Using third party providersection just after we have them activate their environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i moved it inside the third party provider tab but now, i feel uncomfortable with making this warning hidden at first look (need to select the third party provider to read it).
https://docs.upsun.com.fix-generic-gstarted-7jc6g4y-ucq44jg6ofare.eu-5.platformsh.site/get-started/here/make-changes.html#1-create-a-new-environment

and remove the feature branch:

```bash {location="Terminal"}
{{% vendor/cli %}} merge
Copy link
Collaborator

Choose a reason for hiding this comment

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

i'm confused why this was marked resolved since it is still relevant. We're still splitting the path currently between Upsun Git repository and Using third party provider in step 1, so there's still a significant probability that attempting to perform a merge via our cli tool at step 5 will fail when using a 3rd party integration.

Delete sites/friday/http:/127.0.0.1:5001
@chadwcarlson chadwcarlson dismissed gilzow’s stale review February 22, 2024 14:49

Changes have been made

chadwcarlson
chadwcarlson previously approved these changes Feb 22, 2024
@chadwcarlson chadwcarlson merged commit bf972c6 into main Feb 22, 2024
9 checks passed
@chadwcarlson chadwcarlson deleted the fix-generic-gstarted branch February 22, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants