Skip to content
This repository has been archived by the owner on Aug 12, 2022. It is now read-only.

feat!: IgnoreError Recursively for tables and columns #323

Merged
merged 7 commits into from
Jun 8, 2022

Conversation

yevgenypats
Copy link
Member

@yevgenypats yevgenypats commented Jun 7, 2022

πŸŽ‰ Thank you for making CloudQuery awesome by submitting a PR πŸŽ‰

Summary

This adds recursive logic for both columns and tables so we dont have to put it in every column and relation.

This need more tests but ready for initial review.

Closes #323


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines πŸ§‘β€πŸŽ“
  • Run go fmt to format your code πŸ–Š
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests πŸ§ͺ
  • Ensure the status checks below are successful βœ…

@yevgenypats yevgenypats requested a review from a team as a code owner June 7, 2022 11:02
@yevgenypats yevgenypats requested review from bbernays and disq and removed request for a team June 7, 2022 11:02
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

I'll let others reviews comment too, but WDYT about starting with the table level changes, and then moving to the column level changes?

provider/schema/table.go Outdated Show resolved Hide resolved
@yevgenypats yevgenypats force-pushed the feat/ignore_error_recursively branch from be46ca5 to 4c262a6 Compare June 7, 2022 12:22
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Add two more comments

provider/execution/execution.go Outdated Show resolved Hide resolved
provider/execution/execution.go Outdated Show resolved Hide resolved
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

πŸš€ @yevgenypats please squash merge with a commit title prefix of feat!: as this is a breaking change

@yevgenypats
Copy link
Member Author

@erezrokah will do! Will wait on @disq approval (regarding comment) and will squash merge πŸš€

@yevgenypats yevgenypats changed the title feat: IgnoreError Recursively for tables and columns feat!: IgnoreError Recursively for tables and columns Jun 7, 2022
@yevgenypats yevgenypats force-pushed the feat/ignore_error_recursively branch 2 times, most recently from 50a47be to 8622c31 Compare June 8, 2022 11:13
@yevgenypats yevgenypats force-pushed the feat/ignore_error_recursively branch from 8622c31 to a2b1816 Compare June 8, 2022 11:14
Copy link
Member

@disq disq left a comment

Choose a reason for hiding this comment

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

LGTM

@yevgenypats yevgenypats merged commit 7212d98 into main Jun 8, 2022
@yevgenypats yevgenypats deleted the feat/ignore_error_recursively branch June 8, 2022 12:37
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.

None yet

4 participants