Skip to content
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

doc: book migration lost some changes #4906

Closed
tari opened this issue Jan 6, 2018 · 2 comments · Fixed by #4916
Closed

doc: book migration lost some changes #4906

tari opened this issue Jan 6, 2018 · 2 comments · Fixed by #4916

Comments

@tari
Copy link
Contributor

tari commented Jan 6, 2018

Doc structure changes in #4904 appear to have lost some commits that were not correctly kept in sync with the book/ files.

For instance, both f59eb30 and 53012bd only touched manifest.md and not book/src/reference/manifest.md so they were lost in the migration. This is just one issue I was bitten by, so there are likely other commits that were lost as well.

@tari
Copy link
Contributor Author

tari commented Jan 6, 2018

I had a go at automatically finding possibly-problematic commits that didn't update the book.


The book became canonical at ec25b8b (#4453), providing a migration map; I took the final version of it, so it's possible this misses some changes using book file names that changed somewhere in the middle. I want to find commits that touch one of the listed sources but none of the listed destinations.

First get all the commits to master since the book became canonical:

$ git rev-list ^ec25b8b4501a6021f163a07ab19afc9c1660f28e master | tee COMMITS | wc -l
444

Audit each change individually, writing out the commit hashes that appear problematic (script included below):

$ while read COMMIT_HASH
do
	git diff-tree --no-commit-id --name-only -r $COMMIT_HASH \
		| ./audit-doc-changes.py $COMMIT_HASH \
		| tee BAD_COMMITS | wc -l
done <COMMITS
25

The list of possibly-bad commits:


audit-doc-changes.py:

#!/usr/bin/env python3
import sys

# Wildcards manually expanded based on directory contents as of
# 1271bb4de0c0e0a085be239c2418af9c673ffc87
_MIGRATION_MAP = {
        'index.md': {'book/src/index.md',
                     'book/src/getting-started/first-steps.md',
                     'book/src/getting-started/index.md',
                     'book/src/getting-started/installation.md'},
        'guide.md': {'book/src/guide/cargo-toml-vs-cargo-lock.md',
                     'book/src/guide/continuous-integration.md',
                     'book/src/guide/creating-a-new-project.md',
                     'book/src/guide/dependencies.md',
                     'book/src/guide/index.md',
                     'book/src/guide/project-layout.md',
                     'book/src/guide/tests.md',
                     'book/src/guide/why-cargo-exists.md',
                     'book/src/guide/working-on-an-existing-project.md'},
        'build-script.md': 'book/src/reference/build-scripts.md',
        'config.md': 'book/src/reference/config.md',
        'crates-io.md': 'book/src/reference/publishing.md',
        'environment-variables.md': 'book/src/reference/environment-variables.md',
        'external-tools.md': 'book/src/reference/external-tools.md',
        'manifest.md': 'book/src/reference/manifest.md',
        'pkgid-spec.md': 'book/src/reference/pkgid-spec.md',
        'source-replacement.md': 'book/src/reference/source-replacement.md',
        'specifying-dependencies.md': 'book/src/reference/specifying-dependencies.md',
        'faq.md': 'book/src/faq.md',
}

MIGRATION_MAP = {}
for k, v in _MIGRATION_MAP.items():
    if isinstance(v, str):
        v = 'src/doc/' + v
    else:
        v = {'src/doc/' + s for s in v}
    MIGRATION_MAP['src/doc/' + k] = v

commit_hash = sys.argv[1]
changed_files = set(map(str.strip, sys.stdin))

print(changed_files, file=sys.stderr)
problem = False

for changed_file in changed_files:
    must_change = MIGRATION_MAP.get(changed_file, None)
    if must_change is None:
        continue

    if isinstance(must_change, str):
        problem |= must_change not in changed_files
    else:
        problem |= all(f not in changed_files for f in must_change)

if problem:
    print(commit_hash)

@tari
Copy link
Contributor Author

tari commented Jan 8, 2018

Working on cherry-picking the flagged commits:

  • dd09442 patched wording no longer exists
  • 45cc30b updated 3 files under reference
  • 72c9242 update reference/build-scripts.md
  • f9a27fc update reference/manifest.md
  • 82d563b update reference/manifest.md
  • 1240f42 update reference/manifest.md
  • e8a1ba6 formatting fixed
  • 1700346 update reference/manifest.md
  • 7dea9b2 change already present
  • 209513d gcc=0.3 -> cc=1.0
  • 7b65667 typo fixed
  • 723945c update reference/build-scripts.md
  • 4848c30 fixed 'worksapce' misspellings
  • 53012bd update reference/manifest.md
  • 95fa8fe typo fixed
  • 35f13e8 changed wording already exists
  • 32beb11 note already exists
  • 6b94ed2 update reference/manifest.md
  • 8139a5d update reference/manifest.md
  • f59eb30 update reference/manifest.md
  • 13bb7bd created guide/build-cache.md
  • 5b08b8f updated 1 file under reference
  • 11f1443 added heading already exists
  • 2ad45a5 part of a larger sequence of commits that appears to be correct
  • bd5ecd4 false positive; correctly updated book

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 a pull request may close this issue.

1 participant