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.

Sign up for GitHub

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

Jump to bottom

Couldn't we shorten install() #37

Open
amr66 opened this issue Apr 8, 2015 · 4 comments
Open

Couldn't we shorten install() #37

amr66 opened this issue Apr 8, 2015 · 4 comments

Comments

@amr66
Copy link
Contributor

amr66 commented Apr 8, 2015

with get_all_dependencies we get all packages to be installed in an list, odered by dependencies, starting with basic packages and ending with those dependending on the foregoing. So we need not to distinguish between packages[] and requiered[] anymore. Also we clean the list froms doubles, so we needn't unique() twice?
just

  • check packages list
  • without loop: call get_all_dependencies with complete packages list
  • delete packages from list, which are already installed (and have the newest version)
  • loop over list to install...
@maphew
Copy link
Owner

maphew commented Apr 8, 2015

I love when complexity collapses! thanks for pointing this out.

I think we could also move a significant block into remove_installed_from(a_list), and then just call for each of packages and reqs. I added a force=False keyword earlier, with intent of being able to override skipping installed packages, but haven't followed up on that yet. These could be combined nicely.

@amr66
Copy link
Contributor Author

amr66 commented Apr 8, 2015

may be you have a look at my pull request for install() an the use off get_all_dependencies()...

@amr66
Copy link
Contributor Author

amr66 commented Apr 8, 2015

"force" could be used if a package is broken and need's a reinstall. So if force is true we don't check versions but do an install with the localy zipped tar.

maphew added a commit that referenced this issue Apr 9, 2015
@maphew
Merge @amr66 simplifications to install(), #37
c7c6c32 
Only part I didn't use was removal of `reversed(req)`. The intent of
original code here is to install the dependencies in reverse order.
Theoretically this sidesteps any issues of "can't install X because Y
isn't installed yet". I don't see a reason to stop that (but could be
persuaded).
@maphew
Copy link
Owner

maphew commented Apr 9, 2015

in a37a755 discovered that tinyows package lists a non-existent dependency. Install, requires and probably a few others might want some ability to handle broken or incomplete package definitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@maphew @amr66