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

Consolidate DEFAULT_MIN_PROFIT_BNT and DEFAULT_MIN_PROFIT variables #59

Closed
mikewcasale opened this issue Aug 6, 2023 · 0 comments
Closed

Consolidate DEFAULT_MIN_PROFIT_BNT and DEFAULT_MIN_PROFIT variables #59

mikewcasale opened this issue Aug 6, 2023 · 0 comments
Assignees
@Lesigh-3100
Labels
enhancement New feature or request

Comments

@mikewcasale
Copy link
Contributor

Issue
The variables DEFAULT_MIN_PROFIT_BNT and DEFAULT_MIN_PROFIT both refer to the same value. Having these as separate variables could lead to confusion and potential bugs in the code.

Proposed Solution
We should consolidate these two variables into one, using the name DEFAULT_MIN_PROFIT_BNT for clarity that it is denominated in BNT units, and to maintain consistency across the codebase.

Steps

  1. Remove all instances of DEFAULT_MIN_PROFIT.
  2. Replace these instances with DEFAULT_MIN_PROFIT_BNT.
  3. Ensure that all tests still pass after this change.

Additional context
This change will improve code readability and maintenance.
@mikewcasale mikewcasale added the enhancement New feature or request label Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Lesigh-3100 Lesigh-3100

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mikewcasale @Lesigh-3100