Skip to content

Conversation

@markhallen
Copy link
Contributor

What are you trying to accomplish?

This PR implements Phase 2-3 of the group-by: dependency-name feature for grouping dependency updates by dependency name across multiple directories in a monorepo.

Building on #14007, which added the group_by attribute to DependencyGroup, this PR adds the cross-directory handling logic in DependencySnapshot and GroupDependencySelector.

Why: In monorepos with many packages, updating the same dependency (e.g., lodash) across 10 directories creates 10 separate PRs. When group-by: dependency-name is enabled, dependencies need to be marked as handled across ALL directories, not just the current one. This prevents duplicate individual PRs from being created for the same dependency in different directories.

Anything you want to highlight for special attention from reviewers?

  1. DependencySnapshot changes:

    • When group_by_dependency_name? is true, the mark_group_handled method now includes dependencies from existing PRs across ALL directories, not just the current directory
    • This ensures that when we're processing /foo and the group has an existing PR with deps from /foo and /bar, both deps are marked as handled
    • Fixed the type signature for dependencies_in_existing_pr_for_group to correctly return T::Array[T::Hash[String, T.untyped]] instead of T::Array[String]
  2. GroupDependencySelector changes:

    • Added two deduplication strategies in deduplicate_dependencies:
      • group_by_dependency_name? = true: Deduplicate by name only (one entry per dependency name)
      • group_by_dependency_name? = false: Deduplicate by directory + name (original behavior)
    • Extracted common logic into deduplicate_dependencies_with_key which accepts a key extractor block
  3. Backward Compatible: All changes check group.group_by_dependency_name? before applying cross-directory logic, so existing behavior is preserved when the feature is not enabled.

How will you know you've accomplished your goal?

  • ✅ All existing tests pass
  • ✅ New tests added for both DependencySnapshot and GroupDependencySelector
  • ✅ Tests verify cross-directory behavior when group_by_dependency_name? is true
  • ✅ Tests verify original directory-scoped behavior when group_by_dependency_name? is false

Test coverage includes:

DependencySnapshot:

  • Cross-directory inclusion: deps from /bar are marked as handled when processing /foo
  • Empty dependencies edge case handled gracefully
  • Directory filtering preserved when group_by_dependency_name? is false

GroupDependencySelector:

  • Deduplication by name only when group_by_dependency_name? is true (3 unique deps from 4 total)
  • File merging still works correctly (all files from all directories included)
  • Original deduplication by directory+name when group_by_dependency_name? is false (4 deps kept)

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

…endencySnapshot and GroupDependencySelector

This commit adds Phase 2-3 implementation for grouping dependency updates by
dependency name across multiple directories in a monorepo.

Changes:
- DependencySnapshot: When group_by_dependency_name? is true, mark dependencies
  as handled across ALL directories (not just the current one). This prevents
  duplicate individual PRs for the same dependency in different directories.
- GroupDependencySelector: Add deduplication strategies based on group_by setting:
  - group_by_dependency_name?=true: Deduplicate by name only (one entry per dep)
  - group_by_dependency_name?=false: Deduplicate by directory+name (original behavior)
- Fix type signature for dependencies_in_existing_pr_for_group to return
  T::Array[T::Hash[String, T.untyped]] instead of T::Array[String]

All changes are backward compatible and feature-flag gated.
@markhallen markhallen requested a review from a team as a code owner January 29, 2026 16:27
robaiken
robaiken previously approved these changes Jan 30, 2026
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