Skip to content
This repository has been archived by the owner on Jul 19, 2021. It is now read-only.

Add environment to deploy message #866

Merged
merged 3 commits into from
Nov 16, 2018
Merged

Add environment to deploy message #866

merged 3 commits into from
Nov 16, 2018

Conversation

huguestennier
Copy link
Contributor

What are you trying to accomplish with this PR?

Partially fixing #863.

Problem

When Themekit is deploying on Shopify, it will show progress like this :

[environment]: 10 / 30 [=====================>--------------] 33 %

which works great when you are using the default config.yml files.

Since we are using .env files, Themekit doesn't understand that there's multiple environment. So we always show [development] even if we are using another environment.

If we try to add the --env flag to our _generateConfigFlags(), Themekit will complain that there's no 'staging' environment, for example : staging does not exist in this environments list.

Solution

I suggest adding the environment in the ↑ Uploading to Shopify... message above as a workaround.

The developer will now see :

↑  Uploading to Shopify on staging environment...
[development]: 10 / 30 [=====================>--------------]  33 %

I don't think it's perfect, but it works for now.

@huguestennier huguestennier requested a review from t-kelly November 2, 2018 18:58
Copy link
Contributor

@t-kelly t-kelly left a comment

Choose a reason for hiding this comment

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

promiseThemekitConfig() uses themekit to generate a config.yml file. We could generate it ourselves so that we can specify the environment name?

@huguestennier
Copy link
Contributor Author

promiseThemekitConfig() uses themekit to generate a config.yml file. We could generate it ourselves so that we can specify the environment name?

🤦‍♂️ I swear I tried that and it wasn't working. Fixed.

@huguestennier
Copy link
Contributor Author

.env

screen shot 2018-11-12 at 10 40 49 am

.env.staging

screen shot 2018-11-12 at 10 41 08 am

Copy link
Contributor

@t-kelly t-kelly left a comment

Choose a reason for hiding this comment

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

Wicked! 🤙

@lock
Copy link

lock bot commented Dec 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants