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

Breaking change in Package API #65

Open
dilawar opened this issue Nov 12, 2024 · 5 comments
Open

Breaking change in Package API #65

dilawar opened this issue Nov 12, 2024 · 5 comments
Assignees

Comments

@dilawar
Copy link
Collaborator

dilawar commented Nov 12, 2024

This is breaking change. The following code doesn't work anymore.


   --> shepherd-communication\src\push_notification\mod.rs:300:47

    |

300 |         let result = &package_manager.install(mpm::Package::new(package_name, version));

    |                                               ^^^^^^^^^^^^^^^^^               ------- an argument of type `std::string::String` is missing

Originally posted by @dilawar in 14967c9

@ss141309
Copy link
Collaborator

I had introduced package_manager field in the Package struct.
Try doing something like:
let result = &package_manager.install(mpm::Package::new(package_name, &package_manager.pkg_manager_name(), version));

@dilawar
Copy link
Collaborator Author

dilawar commented Nov 12, 2024

That's all fine @ss141309 . I am trying to understand why added this extra field in Package? I missed the reason if it was in previous MRs. I also missed the api change.

Why does Package need to know the PackageManager?

@ss141309
Copy link
Collaborator

In the latest commit related to backing up to and restoring from a TOML/JSON file, we need the package manager's name to determine the source for installing each package.

@dilawar
Copy link
Collaborator Author

dilawar commented Nov 12, 2024

I see. Though I am still confused!

You have a list of packages that you are storing to a json. You want to write the name of package manager along with this list. This is fine. But why individual package needs to keep the name of manager? e.g. { pkg-manager: choco, list_of_packages: [p1, p2...]} vs [ { name: p2, pkg_manager: choco, ..], { name: p2, pkg_manager: choco}, .. ] (not correct json).

The name of package manager can be written by a function like PacakgeManager::save_packages(packages: Vec<Package>)? Still confused why individual package needs to know the name of the package manager!

@ss141309
Copy link
Collaborator

Including the package manager's name for each package now also enables us to list installed and outdated packages across all package managers with a single command, such as mpm list --all

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

No branches or pull requests

2 participants