Various cleanups & fixes

[spoorcc]

Minor cleanups found during implementation of #614

Summary by CodeRabbit

• Documentation

  • Reorganized manual with new dedicated sections for Import, Update, and Diff operations with expanded command documentation.

• Improvements

  • Enhanced reliability of git operations through improved error handling.
  • Improved API clarity with consistent parameter naming conventions.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This PR consolidates documentation updates (docstring clarifications and manual restructuring), refactors parameter naming in the SubProject class, updates test fixtures to use dynamic content, improves error handling in Git test steps, and adds linting suppressions to fuzzing tests.

Changes

Cohort / File(s)	Summary
Documentation & Docstring Updates dfetch/project/git.py, dfetch/project/svn.py	Updated current_revision method docstrings from "revision of the metadata file"/"current revision of the repo" to "last revision of the repository" for consistency.
Method Signature Refactoring dfetch/project/subproject.py	Renamed diff() method parameters from old_rev/new_rev to old_revision/new_revision. Updated internal calls to _diff_impl() and corresponding equality checks. Docstring for current_revision() also updated.
Test Fixture & Step Updates features/steps/svn_steps.py, features/steps/git_steps.py, features/diff-in-svn.feature	SVN server file generation now uses dynamic content "Generated file for {name}" instead of constant "some content". Git test steps replaced subprocess.call with subprocess.check_call for stricter error handling. Feature test patch blocks updated with new content expectations.
Documentation Reorganization doc/manual.rst	Restructured documentation sections: added Getting Started cross-reference, introduced dedicated Import and Update sections with CLI and automodule entries, relocated Validate section under Environment, added new Diff section, and consolidated command documentation.
Linting Configuration tests/test_fuzzing.py	Added module-wide linting suppression pragmas (pyright, mypy, pylint) at file start and removed inline suppress comments from test function calls.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Add fuzzing #928: Modifies the same tests/test_fuzzing.py file with linting pragma adjustments related to fuzzing tests.
• Introduce superproject class #915: Shares parameter/docstring refactoring patterns affecting SubProject, Git, and Svn classes.

Suggested labels

development, documentation

Suggested reviewers

• ben-edna

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Various cleanups & fixes' is vague and generic, using non-descriptive terms that don't convey meaningful information about the specific changes in the changeset.	Consider a more specific title that highlights the primary change, such as 'Rename parameters and update docstrings for consistency' or reference the main category of changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c49de6d and 8afd26c.

📒 Files selected for processing (8)

• dfetch/project/git.py
• dfetch/project/subproject.py
• dfetch/project/svn.py
• doc/manual.rst
• features/diff-in-svn.feature
• features/steps/git_steps.py
• features/steps/svn_steps.py
• tests/test_fuzzing.py

🧰 Additional context used

🧬 Code graph analysis (3)

features/steps/git_steps.py (1)

dfetch/util/util.py (1)

• in_directory (66-75)

features/steps/svn_steps.py (1)

features/steps/generic_steps.py (1)

• generate_file (108-116)

dfetch/project/subproject.py (4)

dfetch/project/git.py (1)

• _diff_impl (54-74)

dfetch/project/svn.py (1)

• _diff_impl (192-208)

tests/test_subproject.py (1)

• _diff_impl (52-53)

dfetch/manifest/project.py (1)

• ignore (440-442)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)

• GitHub Check: build / build (macos-latest)
• GitHub Check: build / build (windows-latest)
• GitHub Check: build / build (ubuntu-latest)
• GitHub Check: Codacy Static Code Analysis
• GitHub Check: run / run (windows-latest, 3.11)
• GitHub Check: run / run (ubuntu-latest, 3.13)
• GitHub Check: run / run (macos-latest, 3.10)
• GitHub Check: run / run (windows-latest, 3.14)
• GitHub Check: run / run (windows-latest, 3.13)
• GitHub Check: run / run (macos-latest, 3.9)
• GitHub Check: run / run (windows-latest, 3.12)
• GitHub Check: run / run (windows-latest, 3.10)
• GitHub Check: run / run (ubuntu-latest, 3.10)
• GitHub Check: run / run (windows-latest, 3.9)
• GitHub Check: test / test
• GitHub Check: run / test-cygwin
• GitHub Check: DevContainer Build & Test

🔇 Additional comments (16)

dfetch/project/git.py (1)

50-52: LGTM!

The docstring clarification correctly distinguishes current_revision() (which returns the repository's last revision) from metadata_revision() (which returns the metadata file's revision). This improves documentation accuracy and aligns with the corresponding change in svn.py.

dfetch/project/svn.py (1)

188-190: LGTM!

The docstring update aligns with the corresponding change in git.py, ensuring consistent terminology across both VCS implementations.

dfetch/project/subproject.py (2)

379-381: LGTM!

The abstract method docstring update maintains consistency with the concrete implementations in GitSubProject and SvnSubProject.

---

404-413: LGTM!

The parameter rename from old_rev/new_rev to old_revision/new_revision improves clarity and aligns with the naming convention already used in the _diff_impl implementations (as shown in the relevant code snippets from git.py and svn.py).

features/steps/svn_steps.py (1)

86-88: LGTM!

The dynamic file content f"Generated file for {name}" aligns with the pattern used in git_steps.py (line 75), ensuring consistent test behavior between Git and SVN feature tests. This makes test artifacts more distinguishable and aids in debugging.

features/steps/git_steps.py (2)

17-35: LGTM!

Switching from subprocess.call to subprocess.check_call improves test robustness by raising CalledProcessError on non-zero exit codes instead of silently continuing. This ensures git command failures are detected immediately, aligning with the existing behavior in svn_steps.py.

---

38-53: LGTM!

The subprocess.check_call usage in tag() and the submodule step ensures that tag creation and submodule operations fail fast on errors, consistent with the rest of the file's changes.

tests/test_fuzzing.py (2)

3-5: LGTM! Clean consolidation of lint suppressions.

Moving the lint suppressions to file-level pragmas is cleaner than having inline comments on each test call. The no-value-for-parameter suppression is appropriate since Hypothesis's @given decorator handles argument injection.

---

197-200: LGTM!

The inline # pylint: disable=no-value-for-parameter comments are no longer needed here since they're now handled by the module-level suppression at the top of the file. This makes the code cleaner and easier to maintain.

features/diff-in-svn.feature (2)

32-33: LGTM! Test expectations aligned with fixture content.

The expected diff content now reflects the actual content generated by the SVN server fixture ("Generated file for SomeProject"), making the test more robust and consistent with how the test infrastructure generates files.

---

82-83: LGTM!

Consistent with the fix in the earlier scenario - the expected base content now matches the fixture-generated file content.

doc/manual.rst (5)

11-11: LGTM!

Good addition of a cross-reference to the Getting Started guide for users who prefer a step-by-step introduction.

---

28-38: LGTM!

The Import section follows the same consistent structure as other command sections. The import_ module name (with underscore) correctly handles Python's import keyword conflict.

---

70-80: LGTM!

The Update section follows the established pattern and is now positioned more logically after Check in the documentation flow.

---

104-114: LGTM!

The Diff section maintains the consistent structure with proper RST heading level and the standard argparse/asciinema/automodule pattern.

---

140-150: LGTM!

The Validate section is well-structured and its placement after Environment makes logical sense as both are related to setup/configuration verification.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}