-
Notifications
You must be signed in to change notification settings - Fork 11
Feature/sprint enhanced velocity report #43
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
Feature/sprint enhanced velocity report #43
Conversation
fivetran-joemarkiewicz
left a comment
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 once we sort out the test error.
fivetran-avinash
left a comment
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 Ready for re-review!
fivetran-joemarkiewicz
left a comment
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.
Looks good, and no need to block approval, but there are a few changes to be made before release review. Please make the below changes or let me know if you have any questions. Once those are applied and docs regenerated, this will be good for release review!
CHANGELOG.md
Outdated
| # dbt_jira_source v0.8.0 | ||
| [PR #43](https://github.com/fivetran/dbt_jira_source/pull/43) introduces the following changes: | ||
|
|
||
| **3 total changes • 3 possible breaking changes** |
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.
Technically in this context the addition of the new is_active fields is not breaking (aka won't cause a disruption in a workflow). We can log these as non-breaking changes.
| **3 total changes • 3 possible breaking changes** | |
| **3 total changes • 0 possible breaking changes** |
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.
Updated.
| {"name": "issue_id", "datatype": dbt.type_int()}, | ||
| {"name": "value", "datatype": dbt.type_string()} | ||
| {"name": "value", "datatype": dbt.type_string()}, | ||
| {"name": "is_active", "datatype": "boolean"}, |
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.
Remove trailing comma.
| {"name": "is_active", "datatype": "boolean"}, | |
| {"name": "is_active", "datatype": "boolean"} |
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.
Removed.
| {"name": "time_zone", "datatype": dbt.type_string()}, | ||
| {"name": "username", "datatype": dbt.type_string()} | ||
| {"name": "username", "datatype": dbt.type_string()}, | ||
| {"name": "is_active", "datatype": "boolean"} |
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.
I noticed the is_active is added here as well as within the src_jira.yml, stg_jira.yml, and CHANGELOG; however, it's not in the stg_jira__user model. Is there a reason this wasn't added there?
If not, please add to the staging model.
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.
Oh geez. Added.
fivetran-avinash
left a comment
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.
Final changes applied, ready for release review.
fivetran-jamie
left a comment
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!
PR Overview
This PR will address the following Issue/Feature: [#36] and
dbt_jira#102This PR will result in the following new package version: v0.8.0
So many new changes.
Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:
Breaking Changes
is_activefield to the following models so customers can more easily find active records.stg_jira__issue_field_historystg_jira__issue_multiselect_historystg_jira__userNew Upstream Models: Sprint Reporting
dbt_jira.Documentation
PR Checklist
Basic Validation
Please acknowledge that you have successfully performed the following commands locally:
Before marking this PR as "ready for review" the following have been applied:
Submission Checklist
Submitter:
See the Height ticket
See Hex workbook linked in the Height ticket
Reviewer:
Changelog
If you had to summarize this PR in an emoji, which would it be?
📿