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

[MAJOR] Remove bundle.environment #72

Merged
merged 2 commits into from
May 4, 2018
Merged

[MAJOR] Remove bundle.environment #72

merged 2 commits into from
May 4, 2018

Conversation

xavdid
Copy link
Contributor

@xavdid xavdid commented Mar 9, 2018

It's been deprecated since the first release, so it's a good a time as any to clean it out and make everything a little more consistent.

It should be noted that I haven't been able to test this directly. It turns out it's a bear to build a zip with a different version of core in it (probably a good thing).

Anyway, the reason I think this is fine is because I've got an app that's been running an app in production that uses an environment variable but that value isn't present in the bundle:

% zapier env 1.0.0
The env of your "Steam" listed below.

┌─────────┬───────────────┬──────────────────────────────────┐
│ Version │ Key           │ Value                            │
├─────────┼───────────────┼──────────────────────────────────┤
│ 1.0.0   │ STEAM_API_KEY │ 8D5A1E0...                       │
└─────────┴───────────────┴──────────────────────────────────┘
│     Log       │ { inputDataRaw: {},                                  │
│               │   meta:                                              │
│               │    { frontend: false,                                │
│               │      standard_poll: true,                            │
│               │      prefill: false,                                 │
│               │      test_poll: false,                               │
│               │      limit: -1,                                      │
│               │      hydrate: true,                                  │
│               │      first_poll: false,                              │
│               │      page: 0 },                                      │
│               │   authData: { steamid: ':censored:15:d607b82756:' }, │
│               │   inputData: {},                                     │
│               │   environment: {} }

the http logs are free of errors, which would only be the case if the api key was correctly set in the environment.

@xavdid xavdid requested a review from eliangcs March 9, 2018 08:55
@xavdid
Copy link
Contributor Author

xavdid commented Mar 9, 2018

Ok! I've confirmed this works.

│     Log       │ { inputDataRaw: {},                                  │
│               │   meta:                                              │
│               │    { frontend: false,                                │
│               │      standard_poll: true,                            │
│               │      prefill: false,                                 │
│               │      test_poll: false,                               │
│               │      limit: -1,                                      │
│               │      hydrate: true,                                  │
│               │      first_poll: false,                              │
│               │      page: 0 },                                      │
│               │   authData: { steamid: ':censored:15:d607b82756:' }, │
│               │   inputData: {} }                                    │
│     Version   │ 1.0.0                                                │
│     Step      │ 740e12ab-cdee-4946-b300-6c9c09cfa241                 │
│     Timestamp │ 2018-03-09T15:46:23-06:00

Copy link
Member

@eliangcs eliangcs left a comment

Choose a reason for hiding this comment

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

I'm giving a 👍in advance, but I don't have historical context about this change. Can you write a little bit more about what event.bundle.environment was and why it was there?

@xavdid
Copy link
Contributor Author

xavdid commented Mar 12, 2018

this actually predates me too! @bryanhelmig might have context. The initial public commit had it marked as deprecated. It seems like they were originally going to pipe the env into the bundle but instead went with it as a proper env variable.

Either way, i'm pretty sure it's vestigial now.

@xavdid xavdid changed the title Remove bundle.environment [MAJOR] Remove bundle.environment Apr 19, 2018
@xavdid xavdid merged commit 63b3bc5 into master May 4, 2018
@xavdid xavdid deleted the remove-bundle-env branch May 4, 2018 20:03
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.

None yet

2 participants