revert changes to DependencyGroupChangeBatch

[jakecoffman]

What are you trying to accomplish?

Reverting #9988 to see if anything breaks. The changes seem like they don't change behavior at all.

I came across this code while investigating rebase command issues.

Anything you want to highlight for special attention from reviewers?

DependencyGroupChangeBatch is only used here:

dependabot-core/updater/lib/dependabot/updater/group_update_creation.rb

Lines 50 to 52 in 9307016

	group_changes = Dependabot::Updater::DependencyGroupChangeBatch.new(
	initial_dependency_files: dependency_snapshot.dependency_files
	)

This method is already in the context of a single directory, so really there should only be one directory in this DependencyGroupChangeBatch. Also the updated_dependencies are never read, except to transform into a DependencyChange which is what gets returned.

The directory is actually set on the dependencies once the DependencyChange is initialized:

dependabot-core/updater/lib/dependabot/dependency_change.rb

Line 63 in 243c481

ensure_dependencies_have_directories

So this is why I think this change isn't needed. I think perhaps the thought here was to propagate it from this point instead of setting it in the DependencyChange.

How will you know you've accomplished your goal?

Nothing breaks!

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.

[jurre]

What exactly does "nothing breaks" mean here? Where would we look for things breaking?

[jakecoffman]

I was hoping to see a test fail. I can't tell how this change makes any functional difference so I'm not sure how exactly to verify that.

From the original PR:

For now this PR should have no effect, even when the feature flag is toggled.

🤷 I guess we didn't need this, I don't see a follow up PR that uses this change

[jakecoffman]

I just confirmed that it's this test which verifies the behavior this fix was trying to solve:

dependabot-core/silent/tests/testdata/vu-group-multidir-all.txt

Lines 92 to 124 in bb8ddc5

	-- input-preexisting.yml --
	job:
	package-manager: "silent"
	source:
	directories:
	- "/foo"
	- "/bar"
	provider: example
	hostname: example.com
	api-endpoint: https://example.com/api/v3
	repo: dependabot/smoke-tests
	dependency-groups:
	- name: all
	rules:
	patterns:
	- "*"
	existing-group-pull-requests:
	- dependency-group-name: all
	dependencies:
	- dependency-name: dependency-a
	dependency-version: 1.2.5
	directory: "/foo"
	- dependency-name: dependency-b
	dependency-version: 2.2.5
	directory: "/foo"
	- dependency-name: dependency-b
	dependency-version: 2.2.5
	directory: "/bar"
	- dependency-name: dependency-c
	dependency-version: 3.2.5
	directory: "/bar"
	experiments:
	dependency_has_directory: true

It still works without this code so therefore this change is not needed.