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

Move SDB into RzUtil #2775

Closed
wants to merge 32 commits into from
Closed

Move SDB into RzUtil #2775

wants to merge 32 commits into from

Conversation

ret2libc
Copy link
Member

@ret2libc ret2libc commented Jul 3, 2022 •
edited
Loading

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the rizin book with the relevant information (if needed)

Detailed description

...

Test plan

  • use dist- branch to make sure source tarball is tested.

Closing issues

...

@ret2libc ret2libc marked this pull request as draft July 3, 2022 12:49
@ret2libc ret2libc force-pushed the sdb-rzutil branch 2 times, most recently from dfd0f95 to 410c9f6 Compare July 3, 2022 12:55
wargio
wargio reviewed Jul 3, 2022
View reviewed changes
wargio
wargio reviewed Jul 3, 2022
View reviewed changes
@XVilka
Copy link
Member

XVilka commented Jul 3, 2022

Please remove JSON first:

@ret2libc
Copy link
Member Author

ret2libc commented Jul 3, 2022

@wargio @XVilka I won't promise I will clean everything in this PR. I think this should be the first step so that future cleaning will be much easier, but please don't expect everything here. I will just remove the obvious stuff, but things that require more changes on "rizin side " will be done in separate PRs later I think.

@XVilka
Copy link
Member

XVilka commented Jul 4, 2022 •
edited
Loading

@ret2libc removing JSON is easy, just merge my PR before and it's done. I don't see a point of importing some files to be removed immediately after, it's better to remove them beforehand.

@ret2libc
Copy link
Member Author

ret2libc commented Jul 4, 2022

If the pr was ready and working why wasn't it merged before? :/

@XVilka
Copy link
Member

XVilka commented Jul 4, 2022 •
edited
Loading

@ret2libc because it was simply forgotten/overlooked. It just requires fixing the test output, and it's ready: #1118 (review)

@ret2libc ret2libc force-pushed the sdb-rzutil branch 6 times, most recently from a0ce318 to 79d5dbb Compare July 4, 2022 17:20
@github-actions github-actions bot added the RzReg label Jul 4, 2022
@ret2libc

This comment was marked as resolved.

Sign in to view
@kazarmy kazarmy requested review from kazarmy and removed request for kazarmy July 9, 2022 15:03
kazarmy
kazarmy reviewed Jul 9, 2022
View reviewed changes
Copy link
Member

@kazarmy kazarmy left a comment

Choose a reason for hiding this comment

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

The ASAN part lgtm

@wargio

This comment was marked as outdated.

Sign in to view
@XVilka
Copy link
Member

XVilka commented Jul 10, 2022

@wargio it was always like that, slightly improving over time: #297

@ret2libc
Copy link
Member Author

ret2libc commented Jul 11, 2022 •
edited
Loading

Closed in favour of #2802

@ret2libc ret2libc mentioned this pull request Jul 11, 2022
Merged
5 tasks
@ret2libc ret2libc closed this Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wargio wargio wargio left review comments

@kazarmy kazarmy kazarmy requested changes

@GustavoLCR GustavoLCR GustavoLCR left review comments

@XVilka XVilka XVilka requested changes

@thestr4ng3r thestr4ng3r Awaiting requested review from thestr4ng3r thestr4ng3r is a code owner

@yossizap yossizap Awaiting requested review from yossizap yossizap will be requested when the pull request is marked ready for review yossizap is a code owner

@Rot127 Rot127 Awaiting requested review from Rot127 Rot127 will be requested when the pull request is marked ready for review Rot127 is a code owner

Assignees
No one assigned
Labels
API infrastructure refactor Refactoring requests rizin rz-asm rz-ax rz-bin rz-diff rz-find rz-gg rz-hash rz-run rz-sign rz-test RzAnalysis RzBin RzBreakpoint RzConfig RzCons RzCore RzCrypto RzDebug RzDiff RzEgg RzFlag RzHash RZIL RzIO RzLang RzMagic RzParse RzReg RzSearch RzSocket RzSyscall RzType RzUtil
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ret2libc @XVilka @wargio @kazarmy @GustavoLCR