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

updated docs #477

Merged
merged 1 commit into from
Jun 30, 2024
Merged

updated docs #477

merged 1 commit into from
Jun 30, 2024

Conversation

grzesir
Copy link
Contributor

@grzesir grzesir commented Jun 30, 2024

No description provided.

Copy link
Contributor

korbit-ai bot commented Jun 30, 2024

My review is in progress 📖 - I will have feedback for you in a few minutes!

@grzesir grzesir merged commit ef735c2 into dev Jun 30, 2024
1 check passed
Copy link
Contributor

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

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

I have reviewed your code and found 2 potential issues. To discuss my individual comments that I have added, tag me in replies using @korbit-ai.


Please react with a 👍 to my comments that you find helpful and a 👎 to those you find unhelpful - this will help me learn and improve as we collaborate.

Comment on lines +58 to +69
html_theme_options = {
"sidebar_hide_name": True,
"light_logo": "logo-light-mode.png",
"dark_logo": "logo-light-mode.png",
"sidebar_title": "Lumibot", # Custom sidebar title
'announcement': """
<div class="footer-banner bg-warning text-dark p-3">
<h5>Need Extra Help?</h5>
<p>Visit <a href="https://www.lumiwealth.com/?utm_source=documentation&utm_medium=referral&utm_campaign=lumibot_footer_banner" target="_blank" class="text-dark"><strong>Lumiwealth</strong></a> for courses, community, and profitable pre-made trading bots.</p>
</div>
"""
}
Copy link
Contributor

Choose a reason for hiding this comment

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

category Documentation

I noticed that there are several lines of code that lack inline comments. While the code is relatively straightforward, adding comments would improve the readability and maintainability of the code. Please consider adding comments to explain the purpose and functionality of the code.

... # If you want to go live, you must change this
... "ENDPOINT": "https://paper-api.alpaca.markets",
... # Set this to False to use a live account
... "PAPER": True
Copy link
Contributor

Choose a reason for hiding this comment

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

category Naming

The variable name 'PAPER' in the Alpaca broker configuration is not descriptive enough. Consider renaming it to 'USE_PAPER_ACCOUNT' to make it clear that it is a flag for using a paper trading account. This will improve code readability and maintainability.

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

Successfully merging this pull request may close these issues.

None yet

1 participant