Skip to content
This repository was archived by the owner on Mar 15, 2018. It is now read-only.

use settings_travis.js when running tests locally (bug 1047109)#544

Closed
cvan wants to merge 1 commit into
mozilla:masterfrom
cvan:use-settings_travis
Closed

use settings_travis.js when running tests locally (bug 1047109)#544
cvan wants to merge 1 commit into
mozilla:masterfrom
cvan:use-settings_travis

Conversation

@cvan

@cvan cvan commented Jul 31, 2014

Copy link
Copy Markdown
Contributor

Comment thread Makefile

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would be good if the test command fails this line is always run so you don't get stuck with unexpected settings.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there no better way we can independently supply settings? Maybe have a settings data attr in the damper served page that overrides any settings here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

what do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like the idea of having a data attr containing a settings object that can override the settings file. We've done that in webpay for a while and we do it in Spartacus too. It works quite nicely and means you can put more service settings in one place.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

how exactly would you do this? put it in src/app.html, src/index.html, and Zamboni's commonplace/index.html?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd start by just putting it where it's needed to start with. In this case we just need damper's html page to override the settings for tests. If we need to override settings from zamboni we can add that to the zamboni html later.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 on that idea.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

can you guys explain to me how exactly that would work? like precisely? thanks

@andymckay

Copy link
Copy Markdown
Contributor

Should we close this pending the feedback?

@cvan cvan closed this Sep 9, 2014
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.

4 participants