Skip to content

Conversation

@Smaug123
Copy link
Contributor

@Smaug123 Smaug123 commented May 3, 2020

This fixes the inconsistency introduced in
4f55250 (#1351), where commit messages were
changed without correspondingly updating the commit hashes.

This is incomplete because I don't yet know how to change the pictures.

The appropriate configuration to use to generate these hashes is:

export GIT_AUTHOR_DATE="2009-05-22T18:09:34+00:00-0700" \
export GIT_AUTHOR_EMAIL="[email protected]" \
export GIT_AUTHOR_NAME="Scott Chacon" \
export GIT_COMMITTER_DATE="2009-05-22T18:09:34+00:00-0700" \
export GIT_COMMITTER_EMAIL="[email protected]" \
export GIT_COMMITTER_NAME="Scott Chacon"

This fixes the inconsistency introduced in
4f55250, where commit messages were
changed without correspondingly updating the commit hashes.
@Smaug123 Smaug123 marked this pull request as draft May 3, 2020 17:16
@Smaug123
Copy link
Contributor Author

Smaug123 commented May 3, 2020

Sigh - there's a yak to start shaving with #1158

@Smaug123
Copy link
Contributor Author

Smaug123 commented May 3, 2020

In fact this has to percolate irritatingly through the rest of this chapter. Would it be better to revert the appropriate section of the original commit? It amounts to changing a couple of commit messages back to lowercase, and the diagrams are already inconsistent (they have the old lowercase commit messages anyway); it would be a lot easier.

@ben
Copy link
Member

ben commented May 6, 2020

I have two feelings about your effort here.

The first is that hey, this is just a book, and it's only meant to be an example. It's not a unit test, the hashes don't necessarily need to match up exactly.

The second is that this is the chapter where we generate hashes, and if you're following along, you should get the same answers we did.

Do we make a promise anywhere in the chapter that the user's answers should match the ones in the book? If so, then by all means, continue along your current path and fix them all. If not, I'd honestly sleep okay at night knowing that the book isn't 100.0% perfect on this. Everything has bugs. 😁

@Smaug123
Copy link
Contributor Author

Smaug123 commented May 7, 2020

The book actually says explicitly that you shouldn't expect to get the same hashes because you're doing it at a different time; but it doesn't mention any other reason one might get different hashes, and I did spend multiple hours trying to work out why my hashes were different.

However, to do the fix properly would be a lot of work. Would you object instead to some sort of line like "The exact commit hashes in this section are representative examples only; while in principle any Git commit object can be reproduced precisely, historical details of this book's construction may mean the printed output does not correspond to a valid set of commits."?

@ben
Copy link
Member

ben commented May 7, 2020

[…] to do the fix properly would be a lot of work. Would you object instead […]

Welcome to the wonderful world of technical book writing. 😄 I would absolutely accept a small note warning people that they may run into discrepancies. Probably want to enclose that in a [NOTE] block.

@Smaug123
Copy link
Contributor Author

Smaug123 commented May 7, 2020

Thanks; I'm closing this in favour of #1424.

@Smaug123 Smaug123 closed this May 7, 2020
@Smaug123 Smaug123 deleted the bugfix-internals-objects-hashes branch May 7, 2020 19:57
@HonkingGoose
Copy link
Contributor

Hi @Smaug123, sorry for making your life difficult because of me changing the commit messages! 🤓

By the way, the hashes will be outdated/wrong later anyways, as Git is going to use SHA-256 hashes instead of the SHA-1 hashes they're using now.
So that would mean all hashes (and pictures with hashes in them) should be replaced to be technically correct again.

https://github.com/git/git/blob/v2.19.0/Documentation/technical/hash-function-transition.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants