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

feat(workers/repository): Update package files in the same order they're extracted #28671

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mbudnek
Copy link
Contributor

@mbudnek mbudnek commented Apr 25, 2024

Changes

For some managers that can have dependencies between input files (pip-compile, for example) it's important to update each input file in the correct order. This changes getUpdatedPackageFiles to sort its lists of package files into the same order that they were returned by the manager's extract function.

Since package files can be referenced by multiple managers and since each manager could return the package files in a different order I also inverted the update logic from

for each package file
    for each manager
        manager.update(package file)

to

for each manager with updated package files
    sortPackageFiles
    for each package file
        manager.update(package file)

This lets us sort the list of package files only once per manager.

Context

#28518

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

For some managers that can have dependencies between input files
(pip-compile, for example) it's important to update each input file in
the correct order.  This changes getUpdatedPackageFiles to sort its
lists of package files into the same order that they were returned by
the manager's extract function.
@mbudnek mbudnek changed the title Update package files in the same order they're extracted feat(workers/repository): Update package files in the same order they're extracted Apr 25, 2024
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type of large core logic change is risky and will take a long time to review/approve/merge

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be carefully reviewed as it's also significant for performance

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.

None yet

3 participants