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

Change template to add platform considerations #5533

Merged
merged 3 commits into from
Oct 9, 2023
Merged

Change template to add platform considerations #5533

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1affc7e
change template to add platform considerations
hjoaquim Oct 8, 2023
5db8d77
Merge branch 'feature/openbb-sdk-v4' into feature/pull-request-template
hjoaquim Oct 9, 2023
1174749
@the-praxs suggestions
hjoaquim Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 56 additions & 8 deletions .github/pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,79 @@
# Description
# Pull Request OpenBB

- [ ] Summary of the change / bug fix.
## The OpenBBTerminal

<details>
<summary> Pull Request the OpenBBTerminal</summary>

hjoaquim marked this conversation as resolved.
Show resolved Hide resolved
### 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?

# 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.
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved

* 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.
- [ ] Make sure affected commands still run in terminal
- [ ] Ensure the SDK still works
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved
- [ ] Check any related reports
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved

### Checklist

# Checklist:

- [ ] I have performed a self-review of my own code.
- [ ] I have commented my code, particularly in hard-to-understand areas.
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved
- [ ] I have adhered to the GitFlow naming convention and my branch name is in the format of `feature/feature-name` or `hotfix/hotfix-name`.
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved
- [ ] Update [our documentation](https://openbb-finance.github.io/OpenBBTerminal/) following [these guidelines](https://github.com/OpenBB-finance/OpenBBTerminal/tree/main/website). Update any user guides that are affected by the changes.
- [ ] Update our tests following [these guidelines](https://github.com/OpenBB-finance/OpenBBTerminal/tree/main/tests).
Copy link
Contributor

Choose a reason for hiding this comment

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

Err... shouldn't we provide the link to our Platform guidelines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we will still need some different guidelines for the Terminal since it will have its own particularities (despite using the Platform).

- [ ] Make sure you are following our [CONTRIBUTING guidelines](https://github.com/OpenBB-finance/OpenBBTerminal/blob/main/CONTRIBUTING.md).
- [ ] If a feature was added make sure to add it to the corresponding [integration test script](https://github.com/OpenBB-finance/OpenBBTerminal/tree/develop/openbb_terminal/miscellaneous/integration_tests_scripts).

</details>

## The OpenBB Platform

<details>
<summary> Pull Request the OpenBB Platform</summary>

hjoaquim marked this conversation as resolved.
Show resolved Hide resolved
### 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.
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved

- [ ] Make sure all unit and integration tests pass
- If you changed a command or added a new one:
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved
- [ ] Make sure affected commands run and the output is correct
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved
- [ ] API
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved
- [ ] Python Interface
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to mention the context of these choices and to choose either or both for the modification/addition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's addressed in the contributing guidelines (which we mention several times here). IMO, we should try to keep this template as short as possible and provide just necessary context for the contributor and the reviewer.

- [ ] If applicable, add new tests for the command (see [CONTRIBUTING.md](/openbb_platform/CONTRIBUTING.md) to leverage semi-automated testing)
- If a new provider was added or a new fetcher was added to an existing provider:
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved
- [ ] Make sure existing tests pass
Copy link
Contributor

Choose a reason for hiding this comment

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

If a new provider/fetcher was added to an existing provider

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, don't agree with this one. sounds weird saying "if a new provider was added to an existing provider"

- [ ] Make sure you can use the new provider and/or fetcher
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved
- [ ] If applicable, add new tests for the provider and/or fetcher (see [CONTRIBUTING.md](/openbb_platform/CONTRIBUTING.md) to leverage semi-automated testing)
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved

hjoaquim marked this conversation as resolved.
Show resolved Hide resolved

### Checklist

# Others
- [ ] I have performed a self-review of my own code.
- [ ] I have commented my code, particularly in hard-to-understand areas.
- [ ] I have adhered to the GitFlow naming convention and my branch name is in the format of `feature/feature-name` or `hotfix/hotfix-name`.
- [ ] Make sure you are following our [CONTRIBUTING guidelines](https://github.com/OpenBB-finance/OpenBBTerminal/blob/main/CONTRIBUTING.md).
- [ ] If applicable, update tests following [these guidelines](/openbb_platform/CONTRIBUTING.md#qa-your-extension).
hjoaquim marked this conversation as resolved.
Show resolved Hide resolved

hjoaquim marked this conversation as resolved.
Show resolved Hide resolved

</details>
Loading