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

Prune dependencies #170

Merged
merged 4 commits into from
Aug 22, 2022
Merged

Prune dependencies #170

merged 4 commits into from
Aug 22, 2022

Conversation

gmarmstrong
Copy link
Contributor

Just removes two unused dependencies (requests and macholib). Fixes #145.

@gmarmstrong gmarmstrong mentioned this pull request Aug 16, 2022
3 tasks
@micahflee micahflee added this to the 0.3.2 milestone Aug 17, 2022
@deeplow
Copy link
Contributor

deeplow commented Aug 18, 2022

A few more deps that can be removed:

dependency reason for introduction
psutil remnants of the dangerzone-vm code, I suspect.
termcolor something about printing things but no longer used
wmi no idea why it was added in the first place. I've asked the developer about it just in case

@gmarmstrong do you think you could remove these as well and rebase it with main so it can be merged?

@gmarmstrong
Copy link
Contributor Author

Oops, apparently "sync fork" does a merge instead of a rebase. Excuse me while I force-push this mistake away...

@gmarmstrong
Copy link
Contributor Author

@gmarmstrong do you think you could remove these as well and rebase it with main so it can be merged?

Done! ✅

@deeplow
Copy link
Contributor

deeplow commented Aug 18, 2022

Thanks a lot!

Oops, apparently "sync fork" does a merge instead of a rebase. Excuse me while I force-push this mistake away...

No worries! You won't beat my struggle with the Github UX where I wanted to open a PR against your fork and accidentally instead opened against the main one, twice! 😅

@deeplow
Copy link
Contributor

deeplow commented Aug 18, 2022

It seems four commits are included from the main branch. I'll try to understand what's going on tomorrow.

@deeplow
Copy link
Contributor

deeplow commented Aug 19, 2022

It should be fixed now! 🙂 I basically did the following to correct it:

  • checkout out your branch
    git checkout --trac gmarmstrong/prune-deps

  • chopped the HEAD 4 times on your branch: 😆 the extra commits that did not belong here
    git reset --hard HEAD^^^^

  • rebased from main
    git rebase main

  • force pushed -- I guess the allow edits by maintainers gives me permissions to push to your fork.
    git push --force

I'm just waiting for Micah's response regarding WMI before merging. Thanks again for your contribution!

@deeplow deeplow merged commit e63c931 into freedomofpress:main Aug 22, 2022
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.

Unneeded dependency: macholib
3 participants