GlacierProtocol / glacierprotocol.github.io

the source for https://glacierprotocol.org
28 stars 27 forks source link

New markdown-based site with support for PDF #4

Closed joaofnfernandes closed 6 years ago

joaofnfernandes commented 6 years ago

This change:

How to test this

There's a make file with targets that allow you to test this locally:

  1. Make sure you have Docker installed
  2. Clone this fork and cd into it
  3. make site starts a local deployment of the website at http://localhost:4000. This should match exactly what you'll get once you host this site with Github Pages
  4. make pdf generates a new pdf based on the markdown source.
diogomonica commented 6 years ago

I was able to review this while it was being developed. LGTM.

bitcoinhodler commented 6 years ago

I would like a few days to review this before it is merged, please.

joaofnfernandes commented 6 years ago

@bitcoinhodler sorry for the mega PR. I tried to include verbose comments so that anyone could contribute to this, but I'm sure there's a couple of areas where the comments might not be good enough.

Let me know if there are any areas that would benefit from more comments/docs and I'm happy to improve this PR.

bitcoinhodler commented 6 years ago

I ran make site and browsed the site locally successfully. But when I subsequently ran make pdf I got some failure from docker:

./_build/generate_pdf.sh
Concatenating all markdown pages into a single one
Unable to find image 'joaofnfernandes/catmd:latest' locally
latest: Pulling from joaofnfernandes/catmd
ff3a5c916c92: Already exists
fa8184a4dbda: Pull complete
Digest: sha256:f6cc9da5df0513f70a563b95bfde3a3f2884b0cbe1810c17d6661986e8d3b44c
Status: Downloaded newer image for joaofnfernandes/catmd:latest
Deploying Glacier website
docker: Error response from daemon: Conflict. The container name "/glacier-website" is already in use by container "5ac97a720695d0d1f814a0693c825b31a80bf06edf4f4773a6a91bfc071d6f76". You have to remove (or rename) that container to be able to reuse that name.
See 'docker run --help'.
Makefile:41: recipe for target 'assets/glacier.pdf' failed
make: *** [assets/glacier.pdf] Error 125

I had to run make run-stop-site to fix this. Seems like there should be a better way.

bitcoinhodler commented 6 years ago

Thank you for the huge improvements in the PDF versus a few weeks ago. Since executing Glacier uses at least 4 computers and many reboots, the printed copy is essential.

But a few nitpicks:

I'll have lots more nitpicks in the next few days.

bitcoinhodler commented 6 years ago

Guys, I think this change needs more discussion on the motivations behind it. While I fully support the move away from Google Docs and into something that can be tracked via Git, I am skeptical on the move away from the PDF as the primary output.

This change does make it easier for random people to contribute changes to the doc, but are the technical issues the real bottleneck there? Due to the security-conscious nature of Glacier, any changes will require careful review. That seems like a bigger bottleneck than any technical difficulties with updating the doc.

For the person who's executing the Glacier protocol, the printed document is of primary importance. I don't think it should be demoted to a second-class citizen as this PR does. The PDF here is clearly a step backward in functionality from the old system.

Let's say we can improve this PR to get the PDF back to the same level of functionality as today's doc. Have we really improved anything? Any future changes will need to consider both the web format as well as the PDF format, making changes more expensive.

Are there other motivations that I've neglected?

diogomonicainc commented 6 years ago

@bitcoinhodler thank you for the comments and the review.

I agree that having a first-class PDF as a deliverable is important since that is the way some people will follow when executing the protocol.

This change is not just focused on making it easier to contribute changes to the protocol (even though it also does that). The major reason is for people to learn about the protocol in an easier fashion. The PDF is a huge barrier to entry/discovery/SEO. Reading the PDF is incredibly painful VS a microsite experience.

The way people engage with Glacier:

I pinged James Hogan on this review like you proposed a few weeks ago. Here's his feedback:

I did take a look at the upcoming glacier website — looks awesome! so much better than a Google Doc pdf! Love what you guys are doing…

Let's make sure we're getting the PDF to a place where we have either quality parity or minor inconveniences over the current version, but I think this is the right direction moving forward.

bitcoinhodler commented 6 years ago

Thank you; that is the explanation I was looking for that was not included in this PR.

As for those executing the protocol -- consulting the protocol online (on a tablet, say) alongside their eternally quarantined hardware is a violation of the protocol and a security risk. You've got your private keys sitting in front of you and there's a camera on that tablet! From section "Prepare Quarantined Workspaces": "Turn off all other computers and smartphones in the room." This is why the first step in the Setup Protocol is to print the document. This also gives the user the opportunity to verify the integrity of the document they're using. Without executing this step and using only the verified document, the user opens himself up to all kinds of new attack vectors.

Glacier is intended to be hard-core about security. We should not let this PR reduce security and we certainly should not be suggesting or encouraging users to do what you're saying. Making it easier for people to reduce their security is a bug, not a feature.

Regarding usability: I find even the online browsing a step backward from the PDF, believe it or not. At the bottom of every page, I have to scroll back up and click on the next section. Even if there was a "Next page" link at the bottom of every page (which would be nice) that's still less usable than hitting the space bar, or page down, to scroll. The inlining of former footnotes is also a step backward in usability.

All that said, I will continue working with the team to help improve Glacier as best as I can. I suspect this PR will require several more iterations once my concerns from May 3 above are addressed.

joaofnfernandes commented 6 years ago

@bitcoinhodler Overall I understand your concerns, and I consider the PDF to be a first-class citizen too. So much so that I lost a lot of time making sure it renders exactly like the site. That's also why I kept the site simple and clean.

But sometimes I was faced with some technical decisions that required a compromise. When that happened I prioritized things with this order in mind:

  1. Generate a site and a PDF from the same source
  2. The PDF needs to look similar and as good as the site
  3. This needs to be easy to maintain

I've tried to address every item in your feedback, and have tracked action items as issues.

Could you spend some time to go through that list of issues and tell me how would you prioritized the issues sorted by most important to least?


Here I'll try to address your comments one by one.

I ran make pdf and got some failure from docker

This is happening because as part of the PDF generation process we actually run the site. This makes sure that what you see in the PDF is as similar as possible as what you see on the site.

The problem is that both the site and PDF end up using the same container name. We can be smarter about this and prevent that error from happening. joaofnfernandes/glacierprotocol.github.io#1

Any way to get page numbers in the TOC?

I really tried to make that happen, but I just don't think it's possible. To understand why, I need to explain you the PDF generation process:

  1. All the content is managed in markdown
  2. The website is generated from markdown
  3. The PDF is generated from the site (with a bit of magic) to make sure it renders exactly like the site.

So I've chosen WeasyPrint as a PDF generator, which converts HTML+CSS to PDF. This is pretty much the same you get when you print to PDF with Chrome. Even if I could find a way to compute at runtime the page where a heading will land, the generator doesn't run javascript, so I'm out of options here.

You might be wondering about the page numbers and heading numbers. Those are done using CSS counters.

Why does section 2 start with 2.5 instead of 2.1?

Seems like I have a bug on my CSS joaofnfernandes/glacierprotocol.github.io#2

Any way to get proper footnotes back, instead of clogging up the main body?

The technical answer is that some flavors of markdown don't support footnotes, including the one I'm using for the site (and in turn for the PDF). And even if possible they don't make sense on a website.

The non-technical answer is that I think footnotes are a crutch that you lean on when you're a lazy writer.

When you're learning the protocol for the first time, footnotes might not have the space to give you more context about things you might want to learn. When you're actually running through the protocol you won't care about those footnotes.

When I engaged in this project I thought that I would rewrite some of the protocol to make it easier to follow. As part of that I would create one or more initial sections that would allow us to bring in all the content that's on footnotes into a centralized place, and expand it with more content. Then the protocol could just link to that section when necessary.

But before I did that, I thought it would be better to focus on the website/PDF first and try to keep things exactly as they are today for the first release of the site.

joaofnfernandes/glacierprotocol.github.io#3

Could the section numbers be included on the web version (instead of just the PDF)? I think it would help communication (when someone reports a problem in "section 2.5")

If your only concern is about people reporting problems, the site has it covered. Each page has a button to file an issue or PR, so maintainers will get all the context needed from there.

These buttons are not easy to find so let's track that joaofnfernandes/glacierprotocol.github.io#4

Version number on title page?

That was an omission. joaofnfernandes/glacierprotocol.github.io#5

In the pdf, there's no mention of the original authors, security advisors, or current maintainers.

My goal for the website+PDF combo is to allow the PDF to excel in the actual protocol execution. The PDF doesn't need to replicate all the information you have on the website and that's useless when you're executing the protocol.

That's why I left this out, I don't think we need to add it back in.

How come the security advisors are no longer listed on the site?

I could not understand if the people listed on the current site are still advisors to the project or not. Maybe @diogomonicainc can clarify this.

Why is "Deposit Protocol" now simply "Deposit"? Yet elsewhere in the doc it's still referred to as "Deposit Protocol". Same for setup/withdrawal/maintenance

When I first started porting this to markdown I did a couple of small edits so that things would be easier for new users. Then I realized that I build port the website+PDF first and do edits later, but never reverted those small changes.

joaofnfernandes/glacierprotocol.github.io#6

Many of the terminal output boxes have newline issues. They look okay on the website, but in the pdf they have extra and/or missing newlines. Example: the raw signed transaction output in the Withdrawal Protocol has both missing and extra newlines.

Can you take screenshots of that and file an issue on my fork so that I can take a closer look into this? I bet it's some CSS issue.

General feedback about not using a tablet while executing the protocol and decreasing security

Absolutely agree. I don't think we are compromising security. We are trying to make things easier to get started, but you still have the PDF so that you can use when actually executing the protocol.

General feedback about usability

We both agree that footnotes are getting in the way. Let's address that as part of joaofnfernandes/glacierprotocol.github.io#3

About making it easy to navigate with space bar. I bet you're a complete power user and navigate the web using

curl https://www.google.com/search?1=weather | less

😂Now seriously... I understand your feedback but most users will be OK with scrolling up and clicking the next section.

I've done a couple of user tests in other contexts, and I've noticed that users don't really click that 'next page' button when it exists. Users won't tell you directly why they don't click it, but if you look at their behavior they always feel a need to understand where they are and where they are going next.

So they prefer clicking on links that are more descriptive and hint at where they're going next. It's true that scrolling up is time consuming, but our pages are not that long, so this won't be a major frustration.

bitcoinhodler commented 6 years ago

Thanks for the detailed reply...I will move detailed discussions to issues on your fork.

diogomonica commented 6 years ago

At this point, I feel like the issues mostly have been addressed.

@joaofnfernandes let's fix the GPG output one that seems higher on the priority list, and we can take the smaller items as we start simplifying the protocol.

diogomonica commented 6 years ago

@joaofnfernandes thanks for the quick turnaround.

@gracenoah any final comments before merge?

gracenoah commented 6 years ago

@diogomonica: I just ran make and make pdf again and everything works, LGTM