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

merge current build context environment vars for lambda build #89

Closed
wants to merge 3 commits into from

Conversation

shmargum
Copy link
Contributor

@shmargum shmargum commented Dec 21, 2018

resolves #59

@8eecf0d2
Copy link
Contributor

Might be worth splitting out to a context merging method to set all options (not just environment) with support for branch specific contexts and the special context branch-deploy. Don't see a need to support deploy-preview since PR's aren't a thing locally and would use the special branch-deploy context if pulled anyway.

Based on docs, given the following config and run within a git branch called foo-branch:

[build]
  publish = "/default"
[build.environment]
  SOME_VAR = true
[context.branch-deploy]
  publish = "/branch-deploy"
[context.branch-deploy.environment]
  SOME_VAR = false
[context.foo-branch]
  publish = "/foo-branch"
[context.foo-branch.environment]
  SOME_OTHER_VAR = 10

We should end up with:

{
  "build": {
    "publish": "/foo-branch",
    "environment": {
      "SOME_VAR": false,
      "SOME_OTHER_VAR": 10,
    }
  }
}

@swyxio
Copy link
Contributor

swyxio commented Dec 29, 2018

sorry for late response - i was on holiday - i see theres also some desire to use the BRANCH env var: #59 (comment) - but i dont see that as a blocker to merge this.

up to your discretion on this one @8eecf0d2, i dont feel too strongly about it

@shmargum
Copy link
Contributor Author

i've updated this to use the branch config
i'd love to add a test but theres no testing framework set up yet

@swyxio
Copy link
Contributor

swyxio commented Dec 31, 2018

awesome - ive been wondering about tests too and should probably get that going at some point. just wasnt built into it from the start.

@shmargum
Copy link
Contributor Author

shmargum commented Jan 2, 2019

I've added some tests as well as a travis yml in #94
don't know if this is what you had in mind; but you can see the travis build successfully running the test on my fork:
shmargum#1
if an admin for this repo enables travis ci then you would also see the tests run on #94

@swyxio
Copy link
Contributor

swyxio commented Jan 2, 2019

wow! nice! very kind of you! unfortunately i'm not an owner of the netlify org so i have to request permission to add travis - just requested it but no guarantees

@shmargum
Copy link
Contributor Author

This is included in #94

@shmargum shmargum closed this Jan 10, 2019
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.

environment context variables do not override build variables
3 participants