-
Notifications
You must be signed in to change notification settings - Fork 39
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
[Pre-release] Bug fix for net income adjustment for no revenue/expense lines #147
[Pre-release] Bug fix for net income adjustment for no revenue/expense lines #147
Conversation
…s in accouning periods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, but are we planning for this to be a pre-release or an official release? If pre-release please make the two changes requested below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fivetran-joemarkiewicz Thanks for the quick checks! This PR is ready for re-review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with the one small adjustment in the README. This will be fine to start the release process for the pre-release tomorrow.
README.md
Outdated
@@ -73,7 +73,7 @@ Include the following QuickBooks package version in your `packages.yml` file. | |||
```yaml | |||
packages: | |||
- package: fivetran/quickbooks | |||
version: [">=0.16.0", "<0.17.0"] # we recommend using ranges to capture non-breaking changes automatically | |||
version: v0.17.0-a1 # we recommend using ranges to capture non-breaking changes automatically |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The official version will not include the v
once it's on the dbt hub.
version: v0.17.0-a1 # we recommend using ranges to capture non-breaking changes automatically | |
version: 0.17.0-a1 # we recommend using ranges to capture non-breaking changes automatically |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Test branch for the following changes. This is a pre-release.
Bug Fix
int_quickbooks__retained_earnings
to ensure accounting periods with no revenue and expense class lines were accounted for.