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

move config to config/content-security-policy.js #104

Merged
merged 3 commits into from
Aug 6, 2019

Conversation

jelhan
Copy link
Collaborator

@jelhan jelhan commented Aug 4, 2019

Moves configuration from ember-cli-build.js to config/content-security-policy.js.

This is mainly motivated by the complexity required to get build-time configuration in different stages of an addon. Build-time config is not available in all hooks and does not seem to be available at all for commands.

This should unblock #103.

@jelhan
Copy link
Collaborator Author

jelhan commented Aug 6, 2019

@sandstrom This was discussed with rwjblue in #103 (comment) cause otherwise the configuration is not accessible from Ember CLI commands. Please raise any concerns as this changes what we had discussed in #92.

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Can you add an example to the README.md for the default configuration? The more thorough example (using mixpanel, google fonts, etc) is great, but it would be nice to also show the bare minimum default config as well.

With this approach we can also add a blueprint that gets ran when ember-cli-content-security-policy is installed that will create a default config/content-security-policy.js file for them.

README.md Show resolved Hide resolved
@@ -25,7 +25,7 @@ ember install ember-cli-content-security-policy

## Configuration

This addon is configured via your application's `ember-cli-build.js` file using `ember-cli-content-security-policy` key:
This addon is configured via `config/content-security-policy.js` file.

- `delivery: string[]`
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about including a typescript style interface here instead of this list? I did something similar over in ember-cli-babel's README, and I personally find it easier to follow than this bulleted list format.

Sorry for the noise in this issue though, we can do whatever changes we want to this bit of the README separately from your PR.

lib/utils.js Outdated Show resolved Hide resolved
jelhan and others added 2 commits August 6, 2019 16:26
Co-Authored-By: Robert Jackson <me@rwjblue.com>
@rwjblue rwjblue merged commit 2324766 into adopted-ember-addons:master Aug 6, 2019
@jelhan jelhan mentioned this pull request Aug 6, 2019
@jelhan
Copy link
Collaborator Author

jelhan commented Aug 6, 2019

@rwjblue Thanks for merging that quickly. Actually I had planed to update the README as suggested but hasn't finished yet. You could find that one in #106.

@jelhan jelhan mentioned this pull request Aug 6, 2019
10 tasks
Copy link
Member

rwjblue commented Aug 7, 2019

Whoops, sorry about that! Seems good to be separate too though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants