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

chore: Do not require dexdump to install package #82

Merged
merged 1 commit into from
Nov 22, 2023
Merged

chore: Do not require dexdump to install package #82

merged 1 commit into from
Nov 22, 2023

Conversation

pnu-s
Copy link
Member

@pnu-s pnu-s commented Oct 30, 2023 •
edited
Loading

Remove dexdump presence check during package installation because, if you have exodus-core in your requirements, it prevents to:

  • install requirements and run tests/lints/etc (workaround: install dexdump even if you don't need it 🤷)
  • install requirements to upgrade them for Dependabot for instance (workaround: none 👎)

@pnu-s pnu-s marked this pull request as ready for review October 30, 2023 16:50
codeurimpulsif
codeurimpulsif approved these changes Nov 1, 2023
View reviewed changes
Copy link
Contributor

@codeurimpulsif codeurimpulsif left a comment

Choose a reason for hiding this comment

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

Just checked the code, seems ok to me.

@U039b
Copy link
Contributor

U039b commented Nov 22, 2023

Why do we still need and use dexdump when analyzing XAPK?

@pnu-s
Copy link
Member Author

pnu-s commented Nov 22, 2023

Discussed offline, we want to remove dexdump but for the time being, let's move this presence check.

@pnu-s pnu-s merged commit 967e65f into v1 Nov 22, 2023
2 checks passed
@pnu-s pnu-s deleted the dexdump_required branch November 22, 2023 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codeurimpulsif codeurimpulsif codeurimpulsif approved these changes

@U039b U039b Awaiting requested review from U039b

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pnu-s @U039b @codeurimpulsif