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

Draft PR: Update package managers (bundler, composer, and npm_and_yarn) to support generalized abstraction #10825

Conversation

kbukum1
Copy link
Contributor

@kbukum1 kbukum1 commented Oct 21, 2024

What are you trying to accomplish?

This PR updates the abstract package manager to include shared attributes like ecosystem, name, version, deprecation_versions, and supported_versions. Additionally, the individual package manager npm_and_yarn is also refactored to extend the abstract package manager and bundler and composer package managers are also updated according to the changes.

Anything you want to highlight for special attention from reviewers?

The goal here is to centralize common functionality into the abstract PackageManagerBase, making each package manager more focused on its unique behavior. If there are any concerns about backward compatibility or potential edge cases in the refactoring, those are key areas to focus on.

How will you know you've accomplished your goal?

  • The package manager refactor should pass all existing tests.
  • The changes should not break any current functionality, and all package managers should behave as expected with the new structure.
  • Any new functionality introduced (e.g., additional attributes in the abstract manager) will have corresponding test coverage.

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.

Commits

  • update abstract package manager to include ecosystem, name, version, deprecation_versions, and supported_versions
  • update bundler package manager according to the changes made in abstract package manager
  • update composer package manager according to the changes made in abstract package manager
  • update npm_and_yarn package manager to support generalized package manager.

@github-actions github-actions bot added L: php:composer Issues and code for Composer L: ruby:bundler RubyGems via bundler L: javascript labels Oct 21, 2024
def unsupported?
# Check if the version is not supported
supported_versions.all? { |supported| supported > version }
super(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved all common fields name, version, deprecation_versions, supported_version into abstract package manager.
Also moved unsupported? function into abstract package manager since bundler and composer both implements unsupported in the same way.

return false if supported_versions.empty?

# Check if the version is not supported
supported_versions.all? { |supported| supported > version }
end
Copy link
Contributor Author

@kbukum1 kbukum1 Oct 21, 2024

Choose a reason for hiding this comment

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

composer and bundler are implementing this method in the same way and other ecosystems are also going to apply in same way. If there is ecosystem needs to implement unsupported in a different way then it should override that in it is package manager

@kbukum1
Copy link
Contributor Author

kbukum1 commented Oct 28, 2024

@kbukum1 kbukum1 closed this Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: javascript L: php:composer Issues and code for Composer L: ruby:bundler RubyGems via bundler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant