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

Updates and clarifications to Readme + Offline app #8

Closed
wants to merge 4 commits into from
Closed

Updates and clarifications to Readme + Offline app #8

wants to merge 4 commits into from

Conversation

ramezrafla
Copy link

No description provided.

Note: We have done some prelim tests and it's working well, pls run your own tests too.
@ramezrafla ramezrafla changed the title Updates and clarifications to Readme Updates and clarifications to Readme + Offline app Jan 16, 2017
@ramezrafla
Copy link
Author

Please ignore the last commit (still working on some edge cases bugs)

@ilan-schemoul
Copy link
Owner

ilan-schemoul commented Jan 16, 2017

Interesting PR, thanks for your participation. Pertinent information have been added to the Readme.
Tough returning HTML is going to cause problems (see comments above).

@ilan-schemoul
Copy link
Owner

ilan-schemoul commented Jan 30, 2017

Okay so I'm just going to take the changes made to the readme.md as you still didn't fix the sw.js

@ramezrafla
Copy link
Author

I am not sure I understand, I am looking at this thread and there is nothing to comment on. What you can do is take what you like from the PR and leave the rest out. Specifically, I am working on sw.js itself. But the readme can use the changes I propose.

@ilan-schemoul
Copy link
Owner

Okay I taught you wanted me to wait you to update the SW.js before doing anything about this PR.

@ilan-schemoul ilan-schemoul reopened this Feb 1, 2017
@ilan-schemoul
Copy link
Owner

Okay I'm going to not merge this PR but just make the needed changes on my local Readme.md then push it. I'm not used to solve PR conflicts so I'm just gonna mess up everything.

@ilan-schemoul
Copy link
Owner

Sorry for the delay I taught for a moment that I've updated the repo already. 8690c88 is based on your update on the Readme.
Thanks for the contribution.

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