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

Remove Instruction.disassembly attribute #540

Merged
merged 9 commits into from
Jan 10, 2025

Conversation

whyitfor
Copy link
Contributor

@whyitfor whyitfor commented Dec 18, 2024

One sentence summary of this PR (This should go in the CHANGELOG!)

  1. Remove the Instruction.disassembly attribute. Its usage is replaced with the existing Instruction.get_assembly(), which is equivalent.
  2. Update Instruction.get_assembly to be synchronous.
  3. ofrak==3.3.0rc0, ofrak_capstone==1.1.0rc0 to ensure versions of these libraries from PyPI are not used. Update makefiles

Link to Related Issue(s)
#539.

Please describe the changes in your request.
See above.

Anyone you think should look at this, specifically?
@dannyp303

@whyitfor whyitfor force-pushed the feature/remove_disassembly_field branch from 64a58c9 to 38d5103 Compare December 18, 2024 02:36
@whyitfor whyitfor requested a review from dannyp303 December 18, 2024 02:40
@whyitfor whyitfor force-pushed the feature/remove_disassembly_field branch from 38d5103 to 0a1faf2 Compare December 18, 2024 03:26
- Existing usages are replace its with `Instruction.get_assembly()`
@whyitfor whyitfor force-pushed the feature/remove_disassembly_field branch from 0a1faf2 to 15ed4c1 Compare December 18, 2024 14:31
@whyitfor whyitfor changed the title Remove Instruction.disassembly attribute Draft: Remove Instruction.disassembly attribute Dec 18, 2024
@whyitfor whyitfor removed the request for review from dannyp303 December 18, 2024 15:35
@whyitfor
Copy link
Contributor Author

Marked as draft for now to address failing tests. One test issue seems to be that the testing packages are importing ofrak_capstone from PyPI.

@whyitfor whyitfor added this to the 3.3.0 Release milestone Jan 8, 2025
@whyitfor whyitfor self-assigned this Jan 8, 2025
@whyitfor whyitfor changed the title Draft: Remove Instruction.disassembly attribute Remove Instruction.disassembly attribute Jan 9, 2025
@rbs-jacob rbs-jacob requested a review from dannyp303 January 9, 2025 16:07
ofrak_core/Makefile Outdated Show resolved Hide resolved
@dannyp303
Copy link
Collaborator

Looks good to me!

@whyitfor whyitfor requested a review from rbs-jacob January 10, 2025 18:42
Copy link
Member

@rbs-jacob rbs-jacob left a comment

Choose a reason for hiding this comment

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

I think these changes make sense. Relying on a pre-release version is probably a habit we should get into in general – we should always have the source version be a release candidate version a bit ahead of the PyPI version.

Good thinking! I also appreciate that this cleans up the code a good amount.

@whyitfor whyitfor merged commit 03c0445 into master Jan 10, 2025
4 checks passed
@whyitfor whyitfor deleted the feature/remove_disassembly_field branch January 10, 2025 22:52
@whyitfor
Copy link
Contributor Author

Closes #539.

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.

3 participants