-
Notifications
You must be signed in to change notification settings - Fork 185
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
navigator.epubReadingSystem "readium build info" data structure #223
Comments
Follow-up: I noticed discrepancies when using "git describe / name-rev --tags" (command line) depending on whether the current build is running from the develop or master tree (e.g. development-mode cloud reader, served locally from Node-express using readium-js-viewer default "grunt" task). So I decided to include the additional "tag" string directly into the navigator.epubReadingSystem "readium" data structure, which saves a third-party manually obtaining the string from the SHA. At troubleshooting time, this will conveniently provide a match between a git commit hash and its nearest recognised tag, even when vendors pull code from develop instead of master (i.e. depending on the git tree, this may indicate an old release tag with a large offset, see JSON + screenshot above). |
I don't think we should bother with the tag in the epubReadingSystem object. The SHA hash of a commit should provide everything we need. Short of someone altering the repo, it will not change and you can easily look it up on github. You can also easily use git describe with the hash as an argument if we wanted to quickly determine the approximate release without spending a few minutes on github. |
@ryanackley I initially thought the same as you: keep it simple, as we can lookup everything we need using the SHA. In practice though (when troubleshooting bug reports, which involves requesting a screenshot or copy/pasting the EPUB report from the affected Readium-based reading system), we would have to open-up a command line shell in each of the repositories to run the "git describe --tags", which is very inconvenient. Having the "human-readable" tag information directly in the report saves lots of time. |
Right. And I fear that many of the less technical QE helpers will not report the number correctly (though ideally they should just grab a screen-shot of the about box or simply copy the hash-value). But I think having a human-readable version is justified for other reasons. Also, it is clumsy to refer to a build/product by the hash. Saying "version 1.16 simply rocks" is a lot easier on the tongue than "version readium-js-viewer@f551ce2956dbde0e411675ba66aa9c55d1cec266, um, rocks!" :-) |
Having a human-created string for the closest tag in the readingsystem Just my 0.02 Patrick On Thu, Oct 16, 2014 at 8:35 AM, Ric Wright [email protected]
Patrick Keating |
My only argument against it is that adds some complexity to the project for what I see as being a very marginal benefit. In the larger scheme of things it's a mole hill. These things slowly add up though so I think it's worth carefully considering. I would add that we've had git hashes in the about box for several months now and I've yet to see a case where anyone has complained about them being too technical. Also, in the rare cases we've used them for something, missing tag information was not a pain point. At least, it wasn't for me and I didn't hear anyone bring it up at the time. |
I hear you, Ryan. Added complexity for little payoff is never a good thing. And I agree that at this point the commit hashes are a more valuable value BUT I think that is going to change. I think that the fruits of all your So I am thinking of how this affects people over the next 12-24 months and Patrick On Thu, Oct 16, 2014 at 11:07 AM, ryanackley [email protected]
Patrick Keating |
I personally think that the additional build script that invokes "git describe" is a small price to pay (i.e. code complexity, grunt maintenance costs) given the benefits (i.e. users can easily report an obvious / meaningful version string for each linked repository, and we don't have to copy/paste + run individual command lines against our local git repo clones). Just to be clear: this is already implemented for the Chrome extension and cloud reader (Grunt build), as illustrated by the screenshot and JSON dump above. As per our conference calls discussions, we are using this platform as our "pilot" implementation, before we kick-start the equivalent development effort in the SDK launcher apps. One possible outcome from this discussion thread (besides blissful collective consensus :) ) is that we decide to not include the additional "tag" information in the navigator.epubReadingSystem object (i.e. preserve only the commit SHA). Should this be the case, we would then remove the Grunt code that invokes "git describe", thereby reducing complexity a little. Personally I think that the Chrome extension / cloud reader "about box" is fine as it is (3x SHA), although I would not vote against including the extra human-readable "tag" as well, if someone requested it (because there will probably be more than one Cloud Reader deployment out there). Let us try to reach an agreement soon on the navigator.epubReadingSystem data structure (field names, etc.), so that we can finally have a (reliable) method to track ReadiumSDK deployments in the wild (of which there are many, considering we are not even at 1.0 yet). And as we know, vendor launcher apps are unlikely to include a useful "about box", so we really need to be able to pull the version info via our special EPUB. |
FYI, now implemented in SDKLauncher-OSX, as a "run script" XCode build phase, in the "epubReadingSystem" feature branch. See the "develop" branch code-diff below (suggestion: scroll all the way down, then read from bottom to top): |
Follow-up on OSX (see above message), Pull Request / feature branch has been merged to develop: |
Follow-up to: #222 - readium/readium-shared-js#104
A final decision needs to be made to approve / amend the proposed data structure in window.navigator.epubReadingSystem that contains information about the Readium build (date, versions, git hashes, etc.). Note that this is currently in the develop branch (as per the merged Pull Request, linked above).
EPUB3 specification:
http://www.idpf.org/epub/301/spec/epub-contentdocs.html#app-epubReadingSystem
Proposed extension (JSON example from current readium-js-viewer (cloud reader / chrome extension)):
Screenshot of corresponding EPUB test (now shipped directly with the default e-books library):
Once the decision is made about the "readium" object format in window.navigator.epubReadingSystem, the same syntax will be adopted for native launcher apps (iOS, OSX, etc. .... although their build system does not extract git hashes just yet, that's a known TODO).
The text was updated successfully, but these errors were encountered: