[WIP] To be rebased into smaller PRs: Improving readability and adding citations: hyperref, cross-references, extra citations; clarifications (e.g. minor rewording and footnotes); list of maths symbols; fixed formatting e.g. subscripts to \mathrm font; archive links; grammar fixes (e.g. complete sentences like in the tables); readme: omitted yellowpaper.io and added more details on how to build.

[jamesray1]

Too big a PR, rebasing to smaller PRs:

• PR URLs for existing refs #373: rebased commits 2 (97b534c) and 3 (e71e30e), and implicitly commit 1.
• PR [WIP] probably needs rebasing into smaller PRs: clarifications, hyperref and grammar #371: Merged 59bcf73, 51b0afb, b2f7605, 08172f4 and 9f03f58 and 60ea686 to local Clarifications branch, but got mixed up with other commits. All commits: numbers 4 (08172f4) to 17 (9f03f58). It's probably better to rebase into still smaller PRs.
• PR \usepackage{hyperref} #370: Merged commits 7 (9072ca8) and 11, (3feff95), and implicitly commit 10 (4710b8e), which was overwritten by 11. Done on Github, but another commit needs to be merge 1e78492
• rebase other commits

[jamesray1]

For the following error in this build see here:

! Package inputenc Error: Unicode char \u8: not set up for use with LaTeX.

[jamesray1]

As expected for a680e16, removing the e with the umlaut didn't help. The build still fails.

[jamesray1]

I just highlighted the white space in the build error, and inspected the element to find that it contains an ellipsis:

However that doesn't seem to mean much as if I highlight non-white space text, e.g. "\u8: not" it shows the same thing.

This ellipsis is also included in the output:

! Package inputenc Error: Unicode char \u8: not set up for use with LaTeX.
See the inputenc package documentation for explanation.
Type H for immediate help.
...

Also if I try to view the page source this is included:

Hey there!

--   |

Looks like you have JavaScript disabled.

  |

The Travis CI webclient needs JavaScript to work properly.

  |

Please enable JavaScript to get the best Travis CI experience.

  |

Thank you!

However, Javascript is enabled in my browser.

I don't get the error when I run ./build.sh in my local repo and the build completes successfully with the file produced.

[axic]

I think you'll have better chances at review and merging this if it is split into smaller pull requests.

[jamesray1]

Maybe you're right, but it will require more work.

[jamesray1]

This is the PDF that produces from my local repo.
Paper.pdf

It produced before all the dependencies that I added on the Github repo were added. I have never got the ! Package inputenc Error: Unicode char \u8: not set up for use with LaTeX. error when building on my local repo.

[jamesray1]

If I split into smaller pull requests I think I would need to have a separate branch for each pull request, or wait until each pull request is merged before making another one.

[jamesray1]

Hooray, bbf6453 got the build to pass! 🎉

[jamesray1]

Ah when I run ./build.sh in my local repo this is included at the start of the output:

This is pdfTeX, Version 3.14159265-2.6-1.40.18 (TeX Live 2017) (preloaded format=latex)
restricted \write18 enabled.
entering extended mode
(/usr/local/texlive/2017/texmf-dist/tex/latex/base/small2e.tex

But the Travis build has:

This is pdfTeX, Version 3.1415926-2.5-1.40.14 (TeX Live 2013/Debian)
restricted \write18 enabled.
entering extended mode
(./Paper.tex
LaTeX2e <2011/06/27>

So clearly TeX Live needs to be updated for this online repo.

Furthermore I also installed the latest version:

This is pdfTeX, Version 3.14159265-2.6-1.40.18 (TeX Live 2017) (preloaded format=latex)
restricted \write18 enabled.
entering extended mode
(/usr/local/texlive/2017/texmf-dist/tex/latex/base/small2e.tex
LaTeX2e <2017-04-15>

[jamesray1]

I couldn't find how to change the "dist" as shown in the Travis CI build config from Trusty to Xenial, which allows installing texlive via the command line. It is not in the .travis.yml file or the travis_deploy.sh file. Perhaps it is a parameter that is set by the Travis CI build version.

Note that there are also options to install the latest version, e.g. here.

So unfortunately it seems that to be able to test what the PDF looks like, we need to install TeX Live 2013/Debian, or upgrade the online Travis CI build to a later OS.

It makes it difficult for viewing the local PDF build which builds differently to the online repo (which is using an outdated LaTeX version, which I can't update, as discussed

[pirapira]

The build is failing again.

[jamesray1]

Closing for now because the references need to be fixed. (There are several Wikipedia and other references that aren't advisable for citing.)

[jamesray1]

I'll leave it open but I will make further changes, but it's not advisable for merging until the references are fixed to be academically suitable.

[jamesray1]

It's been over a week to make changes to the references that took less than half an hour... It probably would've been better to do this earlier but I've had a busy week.

[pirapira]

I cannot review this size of a PR. Please split it into smaller ones.

[jamesray1]

What level of granularity do you require for a PR? In other words, what's the maximum number of lines changed you'd accept for a PR? I don't want to keep having to go back and forth. It's better to know what your criteria are up front.

[pirapira]

I don't have a clearcut standard. PR reviewing is a back-and-forth process. It's harder to review when a PR mixes changes of different purposes (e.g. removing certain kind of references vs. adding hyperlinks).

[jamesray1]

OK I'll have a PR for each item in the list, where possible. But any dependencies will have to go together, such as the cross-references and URLs in the note field in the biblio file are dependent on the hyperref.

[jamesray1]

Merged 59bcf73, b2f7605, 08172f4 to Clarifications branch.

[jamesray1]

My modification for 23c65d3 may not be correct. @pirapira, perhaps you'd like to confirm?

[pirapira]

@jamesray1 no idea about the nibbles and the branches. Sorry.

[jamesray1]

OK it's a bit ambiguous so maybe we should confirm what it means. Maybe Gavin Wood might know, but I'm not sure if I should tag him.

[pirapira]

I think the current sentence is at least hard to read. Will you create an issue about it?

[jamesray1]

I think I already have.

[jamesray1]

#331