Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Expand out offline app features for smoother cache clearing and version tracking #1384
Expand out offline app features for smoother cache clearing and version tracking #1384
Changes from 16 commits
9cece82
392149b
1d165d6
c03f981
526f2de
20aac76
27c7c97
6a6b408
f97411e
56003b3
e5c1faf
64378a6
b0e9ff6
6b9d16c
124ff32
c294505
37979fb
0ba020b
59eebad
be8f8f3
45dff6d
01e9882
47b3a39
839f637
ef337b8
0b34074
384d526
3e10399
3d54f41
a4f0b43
405a3b0
c5b8fc2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, here we can detect if there's a new version, but we don't actually tie this to
getLatestVersionNumber()
, as far as I can read it? How might we do that - could we sayClick to install version X
here? Or isgetLatestVersionNumber()
not reading from the same source asupdatefound
here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have to change things; i like how this looks. But let's leave enough inline comments to make the answer to this question really clear! Esp. because this is complicated stuff, so future coders should be able to follow what you've done. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is optional then please add an
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will GET the latest package JSON. It may not be the one online. We need a better way of GET-ing the version number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By online, do you mean at sequencer.publiclab.org? If so, it looks like we could get the version number from the
stable
branch here on GitHub.Whether we do this depends on whether offline users are mainly working off the
stable
branch or themain
branch. The current implementation of this function is getting the version number from themain
branch, which may be what people are using offline anyways.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For beta.sequencer.publiclab.org, we'll have to get the version from that repo. Beta is on a separate repo btw. For the stable one, it will bw different. Anyway, it will be a mess. Can't we get it from
current_location/package.json
or something? Make it dynamic instead of hard coded?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does the require statement appear twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I think adding all the require statements at the start of the file is a good practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved the require statement to the start of the file, so it's now called only once.