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

Aggregate using TimeSpan #554

Merged
merged 19 commits into from
Sep 11, 2021
Merged

Conversation

heavymanto
Copy link
Contributor

@heavymanto heavymanto commented Sep 6, 2021

Description

Implements #530 to implement `PeriodSize periodsSize = MyTimeSpan.ToPeriodSize();

Checklist

  • My code follows the existing style, code structure, and naming taxonomy
  • I have performed a self-review of my own code and included any verifying manual calculations
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings and running code analysis does not produce any issues
  • I have added or updated unit tests that prove my fix is effective or that my feature works, and achieves sufficient code coverage
  • I have added or run the performance tests that depict optimal execution times
  • New and existing unit tests pass locally and in the build (below) with my changes

Acknowledgements

@DaveSkender
Copy link
Owner

DaveSkender commented Sep 6, 2021

@heavymanto, please restore the description of this PR to include the acknowledgment section. You’ll have to check those boxes to indicate you understand what it means to contribute to an open-source project. You can get it here: .github/PULL_REQUEST_TEMPLATE.md or copy from #545

Copy link
Owner

@DaveSkender DaveSkender left a comment

Choose a reason for hiding this comment

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

A few things to fix:

  • Add full unit test here: tests/indicators/_Common/Test.QuoteHistory.cs. You only tested the external interface.
  • Restore the PR checklist in your description (see prior comment)
  • Once you've done that, I'll add some changes to your branch to update documentation

src/_Common/Functions.cs Outdated Show resolved Hide resolved
src/_Common/Quotes/Quotes.Functions.cs Outdated Show resolved Hide resolved
src/_Common/Quotes/Quotes.Functions.cs Outdated Show resolved Hide resolved
tests/indicators/_Common/Test.Functions.cs Outdated Show resolved Hide resolved
@DaveSkender DaveSkender mentioned this pull request Sep 6, 2021
10 tasks
@DaveSkender DaveSkender changed the title Aggregate time span Aggregate using TimeSpan Sep 6, 2021
@DaveSkender DaveSkender changed the title Aggregate using TimeSpan Aggregate using TimeSpan Sep 6, 2021
@DaveSkender
Copy link
Owner

@heavymanto I'd like to close and merge this PR in the next couple days, can you rebase and take a look at the issues to resolve in the comments above?

@heavymanto
Copy link
Contributor Author

Sorry for the delay. I've had a lot of work to do. I hope my changes are to your liking.

@DaveSkender
Copy link
Owner

DaveSkender commented Sep 11, 2021

Sorry for the delay. I've had a lot of work to do. I hope my changes are to your liking.

Looks pretty good to me. I'll commit a few small items to your branch then merge it (mostly updating docs and fixing merge conflicts).
Thank you for your contribution!

DaveSkender and others added 15 commits September 11, 2021 17:14
* fix internal URLs
* add redirects
* fix internal relative URL refs
* use github repo variable
This Byte Order Mark Unicode character is causing problems in the IDE
* refactor: make ConvertToBasic use enum
* add custom SMA/EMA that can use all OHLCV parts
* fix redirects
* hardcode URLs in contributing since its a native GitHub file
* refactor DPO as a sample case
* update Renko URLs
* add slashes to URLs
* update contributing guide
* move NOTICE file
* lowercase common and src folders
* change header color
* general doc updates
DaveSkender
DaveSkender previously approved these changes Sep 11, 2021
@DaveSkender DaveSkender merged commit d7c1c03 into DaveSkender:main Sep 11, 2021
@github-actions
Copy link

This Pull Request has been automatically locked since there has not been any recent activity after it was closed. Please open a new Issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants