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.

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

Add tests for Global Retention policy in hours #181

Merged
merged 9 commits into from
Sep 19, 2024

Conversation

DHaussermann
Copy link
Contributor

Add tests for Global Retention policy in hours

@DHaussermann DHaussermann self-assigned this Sep 12, 2024
@DHaussermann
Copy link
Contributor Author

@saturninoabril I still have this marked as draft because there are tests in the pipeline failing.
Can you please provide guidance on what's happening here?
image

I'm not even sure if this is in reference to the 2 files I added as part of my PR

Copy link
Member

@saturninoabril saturninoabril left a comment

Choose a reason for hiding this comment

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

There's an error validating the changes. I suggested few items and should passed on validation.

Locally, you may run deno task validate and see it returns All changes are valid.

Screenshot 2024-09-13 at 10 17 55 AM

@DHaussermann
Copy link
Contributor Author

@saturninoabril yes I see. I missed the deno task validate step and went straight to deno task check my mistake.

But, I'm not seeing how tis get's me any closer, the error on my local looks the exact same :)

image

Where is the problem? 0/5 looks like Deno (maybe through a API) is making calls to remote files and something unexpected is in the response. Is this correct or am I misunderstanding the issue?

Yesterday using the Brew package manager I updated Deno. I'm not clear on what I should do next.

@saturninoabril
Copy link
Member

I think you're on track. Have you tried applying the change requests I commented? Once applied, you may try local validation again and see if it fixes the error you're seeing.

DHaussermann and others added 4 commits September 13, 2024 09:56
…-policy/SetGlobalPolicyForAttachmentInHours.md

Co-authored-by: Saturnino Abril <[email protected]>
…-policy/SetGlobalPolicyForAttachmentInHours.md

Co-authored-by: Saturnino Abril <[email protected]>
…-policy/SetGlobalPolicyForMessagesInHours.md

Co-authored-by: Saturnino Abril <[email protected]>
…-policy/SetGlobalPolicyForMessagesInHours.md

Co-authored-by: Saturnino Abril <[email protected]>
@DHaussermann
Copy link
Contributor Author

@saturninoabril Awesome thank man!
image

Locally it's fine now so let's see. The pipeline should say the same.

@DHaussermann DHaussermann marked this pull request as ready for review September 13, 2024 14:22
@DHaussermann
Copy link
Contributor Author

Implemented changes so default status is now correct and removed empty [] causing issue as the array I was trying to provide was not empty.

This is now ready for review.

@DHaussermann
Copy link
Contributor Author

@saturninoabril can you please review once more when you have a chance?

Copy link
Member

@saturninoabril saturninoabril left a comment

Choose a reason for hiding this comment

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

Thanks @DHaussermann, LGTM!

@saturninoabril saturninoabril merged commit 2823184 into main Sep 19, 2024
1 check passed
@saturninoabril saturninoabril deleted the data-retention-hours branch September 19, 2024 13:20
@saturninoabril
Copy link
Member

@DHaussermann Test cases added as MM-T5627 and MM-T5628.

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