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

#2212 Removed hist and trend comamnds from /stocks/ba #2284

Merged
merged 18 commits into from
Sep 17, 2022

Conversation

raviolispy
Copy link
Contributor

@raviolispy raviolispy commented Aug 10, 2022

Removing hist and trend commands from /stocks/ba as per issue Fixes #2212

Description

  • Summary of the change / bug fix.
  • Link # issue, if applicable.
  • Screenshot of the feature or the bug before/after fix, if applicable.
  • Relevant motivation and context.
  • List any dependencies that are required for this change.

How has this been tested?

  • Please describe the tests that you ran to verify your changes.
  • Provide instructions so we can reproduce.
  • Please also list any relevant details for your test configuration.

Checklist:

Others

  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • My code passes all the checks pylint, flake8, black, ... To speed up development you should run pre-commit install.
  • New and existing unit tests pass locally with my changes. You can test this locally using pytest tests/....

@raviolispy
Copy link
Contributor Author

What should I do about docs @colin99d? The command has some crypto info in it but I don't see where to access it from the crypto menu.

@colin99d
Copy link
Contributor

@raviolispy, sorry just saw this. I would just remove the commands from docs.

@raviolispy
Copy link
Contributor Author

Np @colin99d but aren't the commands supported for crypto still?

@colin99d
Copy link
Contributor

Oh I thought they had duplicate versions of the docs in each, good catch. Leave them in!

@raviolispy
Copy link
Contributor Author

Ok so to confirm, no changes to docs?

@colin99d
Copy link
Contributor

Yessir!

@raviolispy
Copy link
Contributor Author

@colin99d hopefully tests pass this time, can you add a label? don't think i have that ability?

@raviolispy raviolispy marked this pull request as ready for review August 21, 2022 19:18
@raviolispy
Copy link
Contributor Author

Just need a review and someone to add a label (or tell me how if I've missed it somewhere) please @colin99d

@jmaslek jmaslek added the enhancement Enhancement label Aug 21, 2022
@colin99d colin99d self-requested a review August 26, 2022 16:43
@colin99d
Copy link
Contributor

Approved! Thanks for this PR.

@jmaslek jmaslek merged commit 3964705 into OpenBB-finance:main Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Sentiment Investor features are dead: Cannot read properties of undefined (reading 'map')"
3 participants