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

[bug] Remove non CF specific files [closes #861] #864

Merged
merged 1 commit into from
Sep 1, 2015

Conversation

codydaig
Copy link
Member

Since the CF env assets doesn't contain anything specific, I stripped it down so it will default to the default settings.

Closes #861

@ryanjbaxter

@codydaig
Copy link
Member Author

Tests are failing waiting on #863. They have nothing to do with this specific PR.

@codydaig
Copy link
Member Author

@jd-carroll Can you try this to see if this resolves your issue?

@lirantal
Copy link
Member

Re-running the CI here to make sure the tests pass.

@mleanos
Copy link
Member

mleanos commented Aug 31, 2015

LGTM

@ryanjbaxter
Copy link
Contributor

@codydaig My original though here was that if you were deploying to CF that you would want to use the minimized version of these files, just like in production. However now that I think about it you may just be doing development and/or testing on CF so maybe it is not necessary. I suppose if you want to run production you can add the files in after the fact.

@jd-carroll
Copy link

That seems to do the trick! I'm a little confused by the change, I would have expected the opposite almost. So if someone could explain a little more why this fixes the issue it would be appreciated.

Also, every time I run cf push, I first have to cut/paste my node_modules folder. Then once done, cut/paste the node_modules folder back into the project. Anyone know why this might be the case?

@ryanjbaxter
Copy link
Contributor

@jd-carroll the problem was that the CF environment file was missing one of the required angular dependencies. By removing all the dependencies from the CF environment file we will inherit the default environment dependencies therefore eliminating the need to maintain dependencies in many different environments.

As for you issue with the node_modules folder, you shouldn't have to do that. In fact the node_modules folder isn't even uploaded to CF when you do the cf push so it should not effect anything. Ping me on Gitter and we can talk more.

@codydaig
Copy link
Member Author

@jd-carroll The configuration of cloud-foundry.js and default.js are merged together and the environment file takes priority. It's smart enough to merge objects deeply, however, when it gets to an array the array in the environment config overwrites the array in the default config. Hope that clarifies things a bit.

@ryanjbaxter That's the difficult part about having cloud-foundry as a seperate enviorment, it doesn't inherit development or production. It is its own env. So yes, once the user decides developemnt or production, they can manually set-up their configuration files.

@lirantal Sounds like this is good to go then.

@lirantal
Copy link
Member

lirantal commented Sep 1, 2015

Great!

@lirantal lirantal added this to the 0.4.x milestone Sep 1, 2015
@lirantal lirantal self-assigned this Sep 1, 2015
lirantal added a commit that referenced this pull request Sep 1, 2015
[bug] Remove non CF specific files [closes #861]
@lirantal lirantal merged commit 40cc691 into meanjs:master Sep 1, 2015
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.

5 participants