Skip to content

Allow replacing default service worker's url - fix #19#52

Merged
TalAter merged 1 commit intoTalAter:masterfrom
mdibaiee:service-worker-url
Dec 30, 2015
Merged

Allow replacing default service worker's url - fix #19#52
TalAter merged 1 commit intoTalAter:masterfrom
mdibaiee:service-worker-url

Conversation

@mdibaiee
Copy link
Contributor

@mdibaiee mdibaiee commented Sep 9, 2015

Hey, I really like the idea of UpUp, I'll try to keep contributing as much as possible ✊

@mdibaiee
Copy link
Contributor Author

@TalAter any feedback on this?

@TalAter TalAter merged commit eef1130 into TalAter:master Dec 30, 2015
TalAter added a commit that referenced this pull request Dec 30, 2015
@TalAter
Copy link
Owner

TalAter commented Dec 30, 2015

Thank you @mdibaiee! Merged 👍

Sorry it took me a while, I was travelling abroad for 3 months.

A couple of comments:

  • I had to change UpUp.addSettings(). When you added service-worker-url to the list of settings, it broke UpUp if users relied on its default and didn't pass it a service-worker-url (it would overwrite the default url with null) -
    if (settings[settingName] !== undefined) {
  • The API docs are generated automatically using markdox. In your next PR (I hope there are more 😄), instead of changing docs/README.md directly, change the comments in the upup.js file... Otherwise your changes will be overwritten automatically when the docs are generated.

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.

2 participants