Skip to content

Feature/remove status field#30

Merged
fivetran-savage merged 7 commits into
mainfrom
feature/remove_status_field
Dec 5, 2025
Merged

Feature/remove status field#30
fivetran-savage merged 7 commits into
mainfrom
feature/remove_status_field

Conversation

@fivetran-savage

Copy link
Copy Markdown
Contributor

PR Overview

Package version introduced in this PR:

  • 1.2.0

This PR addresses the following Issue/Feature(s):

Summary of changes:

  • Removes status field from business_account source, all downstream dependencies, and all documentation.

Submission Checklist

  • Alignment meeting with the reviewer (if needed)
    • Timeline and validation requirements discussed
  • Provide validation details:
    • Validation Steps: Check for unintentional effects (e.g., add/run consistency & integrity tests)
    • Testing Instructions: Confirm the change addresses the issue(s)
    • Focus Areas: Complex logic or queries that need extra attention
  • Merge any relevant open PRs into this PR

Changelog

  • Draft changelog for PR
  • Final changelog for release review

@fivetran-savage fivetran-savage added the docs:ready Triggers the docs generator workflow. label Dec 3, 2025
Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md Outdated
Comment on lines +13 to +14
## Breaking Changes
- The `status` field has been removed from `reddit_ads__account_report`. Since `status` was previously part of the aggregation grain (included in the `GROUP BY`), this change will alter the granularity of the reporting table. Users who previously relied on account-level reporting by status will need to adjust their downstream queries and models accordingly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would consolidate this to the comments for reddit_ads__account_report since it's the same info.

Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md
## Breaking Changes
- The `status` field has been removed from `reddit_ads__account_report`. Since `status` was previously part of the aggregation grain (included in the `GROUP BY`), this change will alter the granularity of the reporting table. Users who previously relied on account-level reporting by status will need to adjust their downstream queries and models accordingly.

## Documentation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor thing--update these to the present tense.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I folded the part you made this comment about into the above definition per another comment. Tenses are kind of mixed throughout changelogs, so let me know if it's still not quite right.

Comment thread CHANGELOG.md Outdated
@fivetran-savage

Copy link
Copy Markdown
Contributor Author

@fivetran-catfritz This should be good to go but let me know if it needs anything else.

@fivetran-catfritz fivetran-catfritz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm!!

@fivetran-savage
fivetran-savage merged commit d162c6f into main Dec 5, 2025
9 checks passed
@fivetran-savage
fivetran-savage deleted the feature/remove_status_field branch December 5, 2025 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs:ready Triggers the docs generator workflow.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants