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

FEATURE: [bollmaker] add emaCross signal #1463

Merged
merged 5 commits into from Dec 22, 2023
Merged

Conversation

c9s
Copy link
Owner

@c9s c9s commented Dec 19, 2023

No description provided.

@bbgokarma-bot
Copy link

Welcome back! @c9s, This pull request may get 248 BBG.

Copy link

codecov bot commented Dec 19, 2023

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (ec4f43b) 21.33% compared to head (382a789) 21.32%.
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1463      +/-   ##
==========================================
- Coverage   21.33%   21.32%   -0.01%     
==========================================
  Files         591      591              
  Lines       43471    43486      +15     
==========================================
  Hits         9274     9274              
- Misses      33522    33537      +15     
  Partials      675      675              
Files Coverage Δ
pkg/strategy/bollmaker/strategy.go 2.69% <0.00%> (-0.12%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec4f43b...382a789. Read the comment docs.

@@ -435,14 +445,20 @@ func (s *Strategy) placeOrders(ctx context.Context, midPrice fixedpoint.Value, k
}
}

if !s.shouldBuy {
Copy link
Collaborator

@zenixls2 zenixls2 Dec 19, 2023

Choose a reason for hiding this comment

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

Not shouldBuy != Should not buy

Maybe rename this variable to shouldNotBuy?

Copy link
Owner Author

Choose a reason for hiding this comment

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

shouldBy is set to true by default, would like to keep it simple to not to put a reversed logic in the variable ^^||

@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 285 BBG

@c9s c9s merged commit c250fec into main Dec 22, 2023
2 of 4 checks passed
@c9s c9s deleted the c9s/bollmaker-ema-crosssignal branch December 22, 2023 17:17
@bbgokarma-bot
Copy link

Hi @c9s,

Well done! 290 BBG has been sent to your polygon wallet. Please check the following tx:

https://polygonscan.com/tx/0x31768629f5d8e4b4cb420fde1777a58f4c8710b5cf792e99afa4be31c86f5b04

Thank you for your contribution!

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.

None yet

5 participants