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

fix/remove duplicated property in PMM strategy #4922

Merged
merged 3 commits into from
Jan 11, 2023

Conversation

takuya29
Copy link
Contributor

Before submitting this PR, please make sure:

  • Your code builds clean without any errors or warnings
  • You are using approved title ("feat/", "fix/", "docs/", "refactor/")

A description of the changes proposed in the pull request:
I found that property and setter of filled_order_delay are doubly defined in pure_market_making.pyx.

Tests performed by the developer:

Tips for QA testing:

@phbrgnomo
Copy link
Contributor

Hello @29Takuya

The Hummingbot Foundation just started the governance process for the community to approve or reject Pull Requests through Pull Request Proposals.

For more information about how PRP votes work, check this page: https://hummingbot.org/governance/prp/

We want to know if you still have interest in having your Pull Request merged into the main codebase.

If so, please check the following on your Pull Request for our team to be able to open it for community voting:

  • Your Pull Request is targeted to the development branch
  • Your branch is up to date with the current development branch
  • All GitHub checks have passed
  • There is no merging conflicts

If you have any questions or need any help, feel free to reach us out on the #dev-general channel on our Discord (https://discord.gg/HAgZ79nzU4)


If you have no interest in merging this PR to our code-base or don't reply to this comment in 30 days, the Foundation team will close this PR.

You can still create a new PR any time you want!

Thank you for contributing to Hummingbot Client!

@takuya29
Copy link
Contributor Author

takuya29 commented Jan 1, 2022

Hi @phbrgnomo,

Thank you for letting me know about the new governance process!

I confirmed that three out of four points are satisfied:

  • Your Pull Request is targeted to the development branch
  • Your branch is up to date with the current development branch
  • All GitHub checks have passed
  • There is no merging conflicts

It seems that GitHub checks don't run on this PR since I am a first-time contributor.
Could you approve this?

@phbrgnomo
Copy link
Contributor

Since this was your first PR, i hade to manually start the tests. Check back later to see if all of them passed.

@takuya29
Copy link
Contributor Author

takuya29 commented Jan 2, 2022

Thank you! I confirmed that all checks have passed.

@RobHBOT
Copy link
Contributor

RobHBOT commented Feb 15, 2022

@29Takuya Takuya did you create a PRP or I can create one for you

@takuya29
Copy link
Contributor Author

@RobHBOT Sorry, not yet. Could you create it on my behalf?

@JeremyKono
Copy link
Contributor

Hi @29Takuya, are you still willing to work on this PR?

@takuya29
Copy link
Contributor Author

Hi @JeremyKono, I am willing to work on this.
The latest development branch still has the same issue.

Would it be possible for you to share what I need to merge this PR?

@cardosofede cardosofede self-assigned this Jun 24, 2022
@cardosofede
Copy link
Contributor

@29Takuya this are the new contribution guidelines https://hummingbot.org/developers/contributions/
Please rebase the code to the current development branch and I will review it.

@takuya29 takuya29 force-pushed the fix/pmm-duplicated-property branch 2 times, most recently from 5875eb2 to 5452bc4 Compare June 24, 2022 22:56
@takuya29
Copy link
Contributor Author

@cardosofede Thank you for your help!
I rebased with the latest development branch.

cardosofede
cardosofede previously approved these changes Jul 25, 2022
Copy link
Contributor

@cardosofede cardosofede left a comment

Choose a reason for hiding this comment

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

LGTM

@nikspz
Copy link
Contributor

nikspz commented Jul 29, 2022

Need to update with latest dev branch changes and good to go

nikspz
nikspz previously approved these changes Jul 29, 2022
Copy link
Contributor

@nikspz nikspz left a comment

Choose a reason for hiding this comment

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

Filled order delay not affected, need to update with latest dev branch and good to go

@takuya29 takuya29 force-pushed the fix/pmm-duplicated-property branch from 5452bc4 to 096bb24 Compare July 30, 2022 07:44
@takuya29
Copy link
Contributor Author

@cardosofede @nikspz Thank you for your reviews! I updated this PR with the latest dev branch.

@JeremyKono JeremyKono dismissed stale reviews from nikspz and cardosofede via 096bb24 August 1, 2022 01:40
Copy link
Contributor

@nikspz nikspz left a comment

Choose a reason for hiding this comment

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

Filled order delay not affected
LGTM

@nikspz nikspz merged commit 7158695 into hummingbot:development Jan 11, 2023
@nikspz
Copy link
Contributor

nikspz commented Jan 11, 2023

In behalf of Foundation team we merged this PR to development for the upcoming Hummingbot version 1.12.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants