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

Rename _target to debug_target for building test cases. #1031

Merged
merged 2 commits into from
Dec 22, 2023

Conversation

r12f
Copy link
Contributor

@r12f r12f commented Dec 21, 2023

Description

This change renames the tests.utils._target function to tests.utils.debug_target

Currently, many tests are required to access the _target function in test_utils for building proper test cases. Hence, it is better to remove the _ prefix for this function, give it a more proper name, really make it public.

This requirement comes from the discussion here: #1028 (comment).

  • My code follows the code style of this project.
  • [-] My change includes a change to the documentation, if required.
  • [-] If my change adds new code, adequate tests have been added.
  • I have read and agree to the CONTRIBUTING document.

@r12f
Copy link
Contributor Author

r12f commented Dec 21, 2023

@Grazfather , as promised below, please let me know how it looks : D

image

@r12f r12f changed the title Rename _target to debug_target for test purpose. Rename _target to debug_target for building test cases. Dec 21, 2023
@hugsy
Copy link
Owner

hugsy commented Dec 22, 2023

@r12f

Great work with all those PRs.

@hugsy hugsy added this to the 2024.01 milestone Dec 22, 2023
@r12f
Copy link
Contributor Author

r12f commented Dec 22, 2023

Thank-you for creating GEF! It is really helpful! Happy to become one of the contributors!

Copy link
Collaborator

@Grazfather Grazfather left a comment

Choose a reason for hiding this comment

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

Just one (albeit annoying) request.

tests/commands/checksec.py Show resolved Hide resolved
@Grazfather Grazfather merged commit 53c769c into hugsy:main Dec 22, 2023
4 of 5 checks passed
@r12f r12f deleted the user/r12f/target branch December 22, 2023 16:57
@r12f
Copy link
Contributor Author

r12f commented Dec 22, 2023

awesome! thanks a lot for the review!

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