Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixed missing handling of the 'ALL' case #574

Merged
merged 3 commits into from
Mar 20, 2023
Merged

Fixed missing handling of the 'ALL' case #574

merged 3 commits into from
Mar 20, 2023

Conversation

why-not-try-calmer
Copy link
Contributor

No description provided.

@why-not-try-calmer why-not-try-calmer force-pushed the fixing-missing-filter branch 2 times, most recently from 4c1e607 to cbe71b2 Compare March 17, 2023 10:20
@suricactus
Copy link
Collaborator

IMO this PR was never tested locally before opening the PR, am I correct?

image

@suricactus suricactus mentioned this pull request Mar 18, 2023
Merged
suricactus
suricactus requested changes Mar 18, 2023
View reviewed changes
Copy link
Collaborator

@suricactus suricactus left a comment

Choose a reason for hiding this comment

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

Please make the fix also for both Job and Delta filter you added recently.

@suricactus suricactus added bug Something isn't working patch Requires patch version change and removed bugfix labels Mar 18, 2023
@suricactus
Copy link
Collaborator

Please use the bug tag instead of bugfix, as the changelog generation is expecting a combination between any of these two:

  • bug | enhancement
  • patch | minor | major

I have removed the OK despite flaky test, because it is not ok yet.

@why-not-try-calmer
Copy link
Contributor Author

why-not-try-calmer commented Mar 20, 2023 •
edited
Loading

IMO this PR was never tested locally before opening the PR, am I correct?

image

Indeed, when it was pushed I was still affected by the typo in the README about billing.SubscriptionPlan that prevented me from launching the stack.

suricactus
suricactus approved these changes Mar 20, 2023
View reviewed changes
Copy link
Collaborator

@suricactus suricactus left a comment

Choose a reason for hiding this comment

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

Code looks good, thank a lot!

@suricactus suricactus merged commit 70898d0 into master Mar 20, 2023
@suricactus suricactus deleted the fixing-missing-filter branch March 20, 2023 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@suricactus suricactus suricactus approved these changes

Assignees
No one assigned
Labels
bug Something isn't working patch Requires patch version change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@why-not-try-calmer @suricactus