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

fix: blank env editor #726

Merged
merged 6 commits into from
Feb 9, 2024
Merged

fix: blank env editor #726

merged 6 commits into from
Feb 9, 2024

Conversation

neurosnap
Copy link
Member

@neurosnap neurosnap commented Feb 8, 2024

Previously we showed all the environment variables which could cause some unintended side-effects and doesn't really map with how environment variables are set in Apps. This change aligns more closely with how the CLI functions: it's additive.

I also updated the explainer text to make it more clear how we parse our textarea.
Screenshot 2024-02-08 at 7 52 25 PM

This maps closer to how it works in the CLI: it's additive.
@fancyremarker
Copy link
Member

Tested functionality; matched expectations. 👍

Added copy improvements:
image

Noticed we show this text on the page, which should only be relevant during FTUX. Did not remove so as not to break behavior elsewhere:

Code scan information is only available for git push deployments.

@fancyremarker
Copy link
Member

Deferring to @almathew for reviewing and deciding on whether to take this approach.

@almathew
Copy link
Contributor

almathew commented Feb 8, 2024

I'll 👍 once we've added Frank's updated language here https://aptible.slack.com/archives/C05CMUW3LHX/p1707434071436489?thread_ts=1707424541.084289&cid=C05CMUW3LHX

@neurosnap neurosnap requested a review from almathew February 9, 2024 00:59
@neurosnap neurosnap merged commit 70d2df4 into main Feb 9, 2024
3 checks passed
@neurosnap neurosnap deleted the blank-env-editor branch February 9, 2024 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants