Switch to asciidoctor 2.0.10 #1373

jnavila · 2020-02-06T22:11:53Z

Change style to new specifications.

reference to #1355

slonopotamus · 2020-02-08T22:49:45Z

Gemfile

@@ -6,8 +6,8 @@ gem 'asciidoctor', '1.5.6.2'
 gem 'json'
 gem 'awesome_print'

-gem 'asciidoctor-epub3', '~> 1.5.0.alpha.9'
-gem 'asciidoctor-pdf', '~> 1.5.0.beta.8'
+gem 'asciidoctor-epub3', '1.5.0.alpha.10'


Please, try alpha-13, it has a bunch of bugs fixed compared to alpha-10 and no known regressions.

OK. Will force-push a corrected version.

HonkingGoose · 2020-02-14T15:33:49Z

Hi @jnavila Should the updated rakefile script be part of this commit? That might help with migrating older pull-requests to the new standard.

Also it might be a good idea to mention in the CONTRIBUTING.md the syntax that should be used for new commits.

jnavila · 2020-08-14T08:49:52Z

Hello,

There's nothing preventing this upgrade, except some thorough check that we are still compatible with the new asciidoc format.

jnavila · 2020-08-28T16:29:58Z

Most pull requests are compatible, I guess, or they can be checked at merging. So I would start by updating and reworking the book in a row, if it's not time-consuming, without dealing with the ongoing PRs. If some have passed during the fixup, just fix them if needed.

I don't remember whether the book is already strict on the new syntax (it's style using the compatibility mode), but I guess asciidoctor 2.0 still supports compat mode, which help split this task in two : bump to 2.x and migrate out of compat mode.

HonkingGoose · 2020-08-28T17:59:19Z

~~We'll first need to get the Travis CI build functional again.~~ Travis CI is functional again! 🎉

Most pull requests are compatible, I guess, or they can be checked at merging. So I would start by updating and reworking the book in a row, if it's not time-consuming, without dealing with the ongoing PRs. If some have passed during the fixup, just fix them if needed.

Doing it in a row is likely to lead to less merge hell than doing the whole book in one shot and then correcting each and every pull at once. And fixing as we go is likely also easier to do in chunks. 😄 So I would follow your process over my suggestions... 👍

I don't remember whether the book is already strict on the new syntax (it's style using the compatibility mode), but I guess asciidoctor 2.0 still supports compat mode, which help split this task in two : bump to 2.x and migrate out of compat mode.

If we can split the task into two parts, it would be easier, first bump to 2.0, turn compat mode on, pin gem dependencies so that we all have the same toolchain. Then migrate away from compat mode. 👍

HonkingGoose · 2020-08-28T21:15:41Z

Well I finally have a working Asciidoctor tool-chain after doing a bundle install that's based on current master. Hallelujah! 🥂 🥳 🎉

I can now actually try to help you with the migration. Would following the Asciidoctor migration cheat sheet be enough to complete the migration? I can help with things like: check if the chapter follows the cheat sheet, click on cross references to check if they still function and other such small tasks.

We still need a way to work together on this. How do you want to organize that?

If you want to use this branch further, you'll need to rebase/merge the master branch, because what is currently on the branch is broken due to the missing kindlegen still being listed in the Gemfile.

I'll let you get us set up and will wait until I hear from you before going off and doing anything. 😄

jnavila · 2020-08-28T21:33:56Z

Well I finally have a working Asciidoctor tool-chain after doing a bundle install that's based on current master. Hallelujah!

I can now actually try to help you with the migration. Would following the Asciidoctor migration cheat sheet be enough to complete the migration? I can help with things like: check if the chapter follows the cheat sheet, click on cross references to check if they still function and other such small tasks.

The migration cheat is all I know. It must be enough.

You seem to be more involved into asciidoctor 2.0. Would you like to update the Gemfiles and check for differences?

Has asciidoctor 2.0 a compat mode?

We still need a way to work together on this. How do you want to organize that?

I think, that we should

start a dev branch
switch to asciidoctor 2.0
make sure all differences from the switch are benign
switch to new mode, and have a check list for all sections.

If you want to use this branch further, you'll need to rebase/merge the master branch, because what is currently on the branch is broken due to the missing kindlegen still being listed in the Gemfile.

I'll let you get us set up and will wait until I hear from you before going off and doing anything.

I've been away for some time, but I can devote some time on this now. Please remember, that's not very high on my priority list, because I am still waiting to hear from a working migration to asciidoctor-pdf 2.0 for Chinese, Japanese and Korean.Until now, we had to use a plug-in which only worked in 1.5.x. I'd like to keep some synchronization between the English version and the translations, plus switching here means accompanying translators on their own migration.

And I'll need to review the impact on git-scm's generation scripts.

jnavila · 2020-08-29T11:08:30Z

On keeping track of everything that needs to be done:

It's already getting quite messy keeping track of everything, and we're only using this issue for the discussion...

I would really, really like to use (and organize) a GitHub Project board. We would then put all our stuff there (to do's, checklists, bugs we need to fix). That way we have a single source of truth .

However to do that on the ProGit2 repo, I would need to be a member of the ProGit2 project, I guess? I would be honored, and would sure want to be a member, but that's really up to the repository owners to decide whether or not I should be.

We can also just use a fork (mine/yours) where we both are collaborators with equal rights and set everything up (dev branch, project board, Travis CI, branch protections, etc) to both our liking there. You can then let Ben pin a issue up top of the ProGit2 issue board with a notice on where to find our project board and dev branch for the migration.

I know little about managing a GitHub project board, but, hey, let's try this.

jnavila · 2020-10-03T19:47:33Z

Hello,

In fact, I already updated my PR: the compilation passes on my laptop, without warnings or errors.

There is still some need for proof-reading, but the overall state seems pretty good. What's your thought on this?

Change style to new specifications.

jnavila · 2020-10-03T20:49:01Z

#1389 can be merged after this one, if the output is correct here.

HonkingGoose · 2020-10-03T20:54:32Z

I'll go try out the stricter set of warnings then, and see if we're really clear of any errors.

HonkingGoose · 2020-10-03T21:01:30Z

Hmm I notice we have a problem with Chinese/Japanese character handling in the PDF file (HTML file is rendering OK).

This is a known issue I think?

because I am still waiting to hear from a working migration to asciidoctor-pdf 2.0 for Chinese, Japanese and Korean.Until now, we had to use a plug-in which only worked in 1.5.x. I'd like to keep some synchronization between the English version and the translations, plus switching here means accompanying translators on their own migration.

~~I'll try a newer version of the gem 'asciidoctor-pdf', '1.5.3' and see if that gets us anywhere.~~
Oh well there is no newer version of asciidoctor-pdf, after making asciidoctor-pdf a wildcard, it still installs version 1.5.3...

jnavila · 2020-10-03T21:30:39Z

One solution would be to request system fonts for this part of rendering. But I'm not versed enough in PDF and asciidoctor-pdf tweaking to achieve this result.

HonkingGoose · 2020-10-03T21:37:28Z

Yeah using Evince as the PDF reader gets me that "tofu" as well. 😄

Shall we just let this slide for now then? It seems to only affect this small part of the book. We can open a issue to track this problem once we have migrated the master branch to asciidoctor 2.

I'm trying my Travis CI build improvements on this branch, and it is complaining, but I suspect that's because of something stupid I'm doing... 😄 I think it's easier to figure this out together once we've done the migration to Asciidoctor 2 instead of cramming it in this pull request as well.

Shall we just freeze our work on this branch, so that I can review the full diff and walk through the book in full?

jnavila · 2020-10-03T21:38:44Z

You're welcome!

HonkingGoose · 2020-10-03T21:40:03Z

I think it was already the case before. The required fonts (Chinese and another one I don't remember) are not included in the PDF.

The PDF not rendering properly is indeed a old issue, so it's not something we're breaking with this migration, as it was broken to begin with. 😄 #977 (comment)

You're welcome!

I'll go do a full review of the diff tomorrow, and compare the HTML files between master and this branch.

jnavila · 2020-10-03T21:49:46Z

I'll go do a full review of the diff tomorrow, and compare the HTML files between master and this branch.

🙏

HonkingGoose · 2020-10-04T07:45:16Z

HonkingGoose · 2020-10-04T08:32:14Z

book/07-git-tools/git-credential-read-only

@@ -15,7 +15,7 @@ exit(0) unless File.exists? path

 known = {} # <3>
 while line = STDIN.gets
-    break if line.strip == ''
+    break if line.strip == `"


Suggested change

break if line.strip == `"

break if line.strip == ``

I don't think we should be migrating this code?

True. My script is blind to the context and has also applied to code blocks. How does it render?

It renders OK, in the HTML output:

I still think we should change it back to what it was previously. I don't know if '" is even valid to use with Ruby code.

This isn't correct. The hunk must be completely reverted to the original line.

HonkingGoose · 2020-10-04T08:32:58Z

TRANSLATING.md

@@ -59,7 +59,7 @@ If there is no project for your language, you can start your own translation.
 Base your work on the second edition of the book, available [here](https://github.com/progit/progit2). To do so:
 1. Pick the correct [ISO 639 code](https://en.wikipedia.org/wiki/List_of_ISO_639-1_codes) for your language.
 1. Create a [GitHub organization](https://docs.github.com/en/github/setting-up-and-managing-organizations-and-teams/creating-a-new-organization-from-scratch), for example: `progit2-[your code]` on GitHub.
- 1. Create a project ``progit2``.
+ 1. Create a project "`progit2"`.


Suggested change

1. Create a project "`progit2"`.

1. Create a project `progit2`.

Undo changes the migration script has made, as this is not valid Markdown.

I think the correct change is:

- 1. Create a project ``progit2``. + 1. Create a project "`progit2`".

That would be right if this was a AsciiDoctor file, but we're in a Markdown file.

Your proposal doesn't render properly in Markdown:

Create a project "`progit2'"

Rendering preview:

Left is source, right is Markdown preview:

Oh, ah. It's markdown here… Let's keep it as it was before the patch.

book/07-git-tools/sections/rewriting-history.asc

HonkingGoose

There are some changes we need to make.

HonkingGoose · 2020-10-04T09:01:25Z

We also have problems with some images, but those problems are also present on our current master.
I'll go make an issue for that once we're done migrating.

Check out the diagram that comes with Figure 6. Working tree, staging area, and Git directory. The ends of the arrow are way too big. Figure 27. A “silo” view of progressive-stability branching has the same issue as well.

This seems to be a known issue, compare Ben's output with what we have currently on master and on this branch.
#1288 (comment) Ben's image is captioned: Figure 8. The lifecycle of the status of your files.

HonkingGoose · 2020-10-04T09:38:48Z

I've skimmed the entire book and compared many sections against the html files on master.
I'm reasonably sure that I found the major issues now.

HonkingGoose · 2020-10-04T10:05:29Z

Checklist before merge:

Fix bug with yellow text. (jnavilla and honkinggoose), see pull Fix highlighted text in AsciiDoctor 2 migration branch. #1536.
Implement review feedback on the GitHub full diff. (jnavilla) See pull Fix 2 outstanding review issues for Asciidoctor 2 migration #1541
Get a review from the head honcho's here. (ben).
Ask about impact on git-scm generation scripts. See Preventing problems with git-scm for Asciidoctor 2 and version bump to ProGit2 book generation. git/git-scm.com#1516 (I've asked, we're good to go as-is. 😄 )
Decide on version number strategy (jnavilla, ben)
If a version bump is wanted, arrange the bump in this pull request.

Discussion: version number strategy

Most software follows the: major.minor.patch version number strategy.
I think a "breaking change" such as this one merits a version bump.
Even more so because existing pull request will not merge cleanly on the newer version due to syntax changes.

If we were following semantic versioning it even merits a 3.0.0 release, as we making a "breaking change" in the way the book is generated. I understand and agree that for branding/marketing purposes a 3.0.0 release is for a major rewrite of the book. Readers will be disappointed if we release a 3.0.0 and it's not a major rewrite. So I propose do a minor version bump from 2.1.x to 2.2.x.

To make this change, we need to edit the following files:

progit2/script/tag_on_master

Lines 1 to 19 in a8ec5ac

    
           #!/bin/bash 
        
           # This is for running on Travis. It automatically tags any merge to Master as a release in the 2.1.x series. 
        
           if [[ $TRAVIS_PULL_REQUEST != 'false' || "$TRAVIS_BRANCH" != 'master' ]]; then 
        
             # Don't run on pull requests 
        
             echo 'This only runs on a merge to master.' 
        
             exit 0 
        
           fi 
        
           # Compute the next tag number 
        
           LASTPATCH=$(git describe --tags | cut -d- -f1 | cut -d. -f3) 
        
           PATCH=$(($LASTPATCH+1)) 
        
           echo $PATCH 
        
           # Create a tag 
        
           curl -H "Authorization: token $GITHUB_KEY" \ 
        
                -X POST \ 
        
                -d "{\"ref\":\"refs/tags/2.1.$PATCH\", \"sha\":\"$TRAVIS_COMMIT\"}" \ 
        
                https://api.github.com/repos/progit/progit2/git/refs

progit2/.travis.yml

Lines 26 to 29 in a8ec5ac

    
           branches: 
        
             only: 
        
             - master 
        
             - /^2\.1(\.\d+)+$/

Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com>

jnavila · 2020-10-04T15:54:43Z

I didn't know that the # sign is a marker for highlighting. It seems a bit tricky but We may change these occurrences to +#3+.

As for version number, right now this is just an image of the edition number (second edition) + a build number. This is not optimal, but it's enough to tell book versions apart. Bumping to 3 seems odd, because this isn't really a rewrite of the book. It's an internal number so, we could just bump to a larger build number. Anyway, this won't help for upcoming pull requests.

HonkingGoose · 2020-10-04T18:43:44Z

Agree, bumping to 3 is not the right thing to do, as this is not a rewrite.

Do you want to bump the minor version from 2.1.x to 2.2.x?
Or do you want to keep using the current versioning scheme?

I'm not sure I understand what you mean with:

It's an internal number so, we could just bump to a larger build number.

Fix highlighted text in AsciiDoctor 2 migration branch.

HonkingGoose · 2020-10-05T11:21:42Z

There are also still some items we need to do from the checklist #1373 (comment). Can you take a look and respond to those items?

HonkingGoose · 2020-10-18T07:53:51Z

Hi @jnavila can you merge in #1541?

I checked with git-scm we're good to go with them.
After you've merged my pull, we can request a review from Ben.

HonkingGoose · 2020-10-22T19:43:59Z

Hi @ben, I thought I would write a quick status report for you to catch up, as a lot has happened on this pull request.

This migration is nearly ready to go, I still need Fix 2 outstanding review issues for Asciidoctor 2 migration #1541 to be merged into this branch to fix 2 small issues with the automated migration.
We should not be causing issue with the git-scm website when we migrate to Asciidoctor 2.0.10 (git-scm already uses Aciidoctor 2 on their end).
I skimmed the full book, and found no egregious errors/breakage.
I pinned all the dependencies, so that Dependabot will help us update those in the future.
I think it's a good idea to do a squash-merge on the final pull request, that way you only have one commit to revert instead of multiple in case something goes badly wrong.

I do want your feedback on whether or not you think this merits a minor version bump (from 2.1.x to 2.2.x)? If you do want a version bump, I'll need to change some files to bump the versioning automation.

Greetings,

HonkingGoose

ben · 2020-10-22T21:05:49Z

Thanks so much to you both for working through this! I'll owe you 🍻 if we ever happen to be in the same place.

I actually don't have a preference as to what kind of merge happens here. GitHub makes it pretty easy to revert an entire PR, and we haven't been historically picky about the shape of the history. If you feel like there'll be too much noise when using Git to browse the history, then by all means do a squash.

slonopotamus reviewed Feb 8, 2020

View reviewed changes

slonopotamus mentioned this pull request Feb 29, 2020

Bump asciidoctor-epub3 to 1.5.0.alpha.14 #1388

Closed

HonkingGoose mentioned this pull request Jul 2, 2020

Update asciidoctor requirement from 1.5.6.2 to 2.0.10 #1454

Closed

HonkingGoose mentioned this pull request Aug 9, 2020

Proposal: use versions for other Ruby gems too. #1358

Closed

This comment has been minimized.

Sign in to view

This was referenced Aug 21, 2020

Travis CI build does not fail on Asciidoctor warning #1491

Closed

Breaking builds: Kindlegen dependency no longer available #1496

Closed

This comment has been minimized.

Sign in to view

jnavila force-pushed the asciidoctor_2.0.10 branch from 5b1b34b to 3914dc5 Compare September 16, 2020 19:19

This comment has been minimized.

Sign in to view

jnavila force-pushed the asciidoctor_2.0.10 branch from 3914dc5 to ce427ff Compare October 3, 2020 19:43

This comment has been minimized.

Sign in to view

jnavila and others added 2 commits October 3, 2020 22:45

Switch to asciidoctor 2.0.10

b878b3f

Change style to new specifications.

Pin dependencies

a8ec5ac

jnavila force-pushed the asciidoctor_2.0.10 branch from 6d77fbe to a8ec5ac Compare October 3, 2020 20:46

HonkingGoose reviewed Oct 4, 2020

View reviewed changes

book/07-git-tools/sections/rewriting-history.asc Outdated Show resolved Hide resolved

HonkingGoose suggested changes Oct 4, 2020

View reviewed changes

This comment has been minimized.

Sign in to view

Update book/07-git-tools/sections/rewriting-history.asc

8897bab

Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com>

HonkingGoose approved these changes Oct 4, 2020

View reviewed changes

Fix unintended highlighting of text

bfa64b7

HonkingGoose mentioned this pull request Oct 4, 2020

Fix highlighted text in AsciiDoctor 2 migration branch. #1536

Merged

2 tasks

Merge pull request #1536 from HonkingGoose/patch-1

aaaab2b

Fix highlighted text in AsciiDoctor 2 migration branch.

HonkingGoose approved these changes Oct 5, 2020

View reviewed changes

This was referenced Oct 11, 2020

Fix 2 outstanding review issues for Asciidoctor 2 migration #1541

Closed

Preventing problems with git-scm for Asciidoctor 2 and version bump to ProGit2 book generation. git/git-scm.com#1516

Closed

HonkingGoose mentioned this pull request Nov 3, 2020

Migrate to Asciidoctor 2.0.11 #1554

Merged

2 tasks

jnavila closed this in #1554 Nov 6, 2020

	1. Create a project "`progit2"`.
	1. Create a project `progit2`.

Switch to asciidoctor 2.0.10 #1373

Switch to asciidoctor 2.0.10 #1373

Conversation

jnavila commented Feb 6, 2020

slonopotamus Feb 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HonkingGoose commented Feb 14, 2020

This comment has been minimized.

jnavila commented Aug 14, 2020

This comment has been minimized.

jnavila commented Aug 28, 2020

HonkingGoose commented Aug 28, 2020 • edited Loading

HonkingGoose commented Aug 28, 2020

jnavila commented Aug 28, 2020

This comment has been minimized.

jnavila commented Aug 29, 2020

On keeping track of everything that needs to be done:

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jnavila commented Oct 3, 2020

This comment has been minimized.

This comment has been minimized.

jnavila commented Oct 3, 2020 • edited Loading

HonkingGoose commented Oct 3, 2020

HonkingGoose commented Oct 3, 2020 • edited Loading

jnavila commented Oct 3, 2020

HonkingGoose commented Oct 3, 2020

jnavila commented Oct 3, 2020

HonkingGoose commented Oct 3, 2020 • edited Loading

jnavila commented Oct 3, 2020

HonkingGoose commented Oct 4, 2020 • edited Loading

HonkingGoose's review progress:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HonkingGoose Oct 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rendering preview:

Choose a reason for hiding this comment

HonkingGoose left a comment

Choose a reason for hiding this comment

HonkingGoose commented Oct 4, 2020 • edited Loading

This comment has been minimized.

HonkingGoose commented Oct 4, 2020

HonkingGoose commented Oct 4, 2020 • edited Loading

Checklist before merge:

Discussion: version number strategy

jnavila commented Oct 4, 2020

HonkingGoose commented Oct 4, 2020

HonkingGoose commented Oct 5, 2020 • edited Loading

HonkingGoose commented Oct 18, 2020

HonkingGoose commented Oct 22, 2020

ben commented Oct 22, 2020

slonopotamus Feb 8, 2020 •

edited

Loading

HonkingGoose commented Aug 28, 2020 •

edited

Loading

jnavila commented Oct 3, 2020 •

edited

Loading

HonkingGoose commented Oct 3, 2020 •

edited

Loading

HonkingGoose commented Oct 3, 2020 •

edited

Loading

HonkingGoose commented Oct 4, 2020 •

edited

Loading

HonkingGoose Oct 4, 2020 •

edited

Loading

HonkingGoose commented Oct 4, 2020 •

edited

Loading

HonkingGoose commented Oct 4, 2020 •

edited

Loading

HonkingGoose commented Oct 5, 2020 •

edited

Loading