Skip to content

Conversation

@shimonp21
Copy link
Contributor

Summary


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 βœ…

@shimonp21 shimonp21 requested a review from yevgenypats as a code owner October 7, 2022 11:15
@github-actions github-actions bot added the fix label Oct 7, 2022
@shimonp21 shimonp21 enabled auto-merge (squash) October 7, 2022 11:16
Copy link
Contributor

@yevgenypats yevgenypats left a comment

Choose a reason for hiding this comment

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

looks good but I think there is a bug

for r := range res {
if _, ok := r.Data[schema.CqSourceName.Name]; ok {
r.Data[schema.CqSourceName.Name] = source
if _, ok := r.Data[schema.CqSourceName.Name]; !ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

why this is changed from ok to !ok ? if it doesn't exist it all then the write query will fail because this column wasn't migrated. but it should just be there and be nil and we should overwrite it.

@shimonp21
Copy link
Contributor Author

replaced by #269

@shimonp21 shimonp21 closed this Oct 10, 2022
auto-merge was automatically disabled October 10, 2022 08:31

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants