Skip to content

chore: bump deps #385

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore: bump deps #385

wants to merge 1 commit into from

Conversation

Pi-Cla
Copy link

@Pi-Cla Pi-Cla commented Nov 19, 2024

Not sure what to do in wait_for_event() in test.rs when the item returns a RedisError. For now opted to just unwrap() since this is a test.

jxl_oxide has deprecated Render::image() in favour of Render::stream()

Comment out unused redis22

In Rocket, Outcome::Failure has become Outcome::Error

to_rfc3339_string() is deprecated in favour of it's equivalent: try_to_rfc3339_string().unwrap() This should be ok because we are calling this method on DateTime::now()

Please make sure to check the following tasks before opening and submitting a PR

  • I understand and have followed the contribution guide
  • I have tested my changes locally and they are working as intended

Not sure what to do in wait_for_event() in test.rs when the item returns a RedisError. For now opted to just unwrap() since this is a test.

jxl_oxide has deprecated Render::image() in favour of Render::stream()

Comment out unused redis22

In Rocket, Outcome::Failure has become Outcome::Error

to_rfc3339_string() is deprecated in favour of it's equivalent: try_to_rfc3339_string().unwrap()
This should be ok because we are calling this method on DateTime::now()

Signed-off-by: Pi-Cla <[email protected]>
@Pi-Cla
Copy link
Author

Pi-Cla commented Nov 20, 2024

Force pushed to ensure that this builds on Rust 1.77

@insertish insertish self-requested a review April 25, 2025 10:51
Copy link
Member

@insertish insertish left a comment

Choose a reason for hiding this comment

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

LGTM!
Very sorry for late review, I was very busy this month.
Some changes from other team members are now conflicting with yours.

We're now also dropping the 1.77 requirement and actively trying to fix the issue causing the perf. regression so no need to worry about that.

@@ -101,6 +101,9 @@ impl TestHarness {

let mut stream = self.sub.on_message();
while let Some(item) = stream.next().await {
// Not sure what should be done if we get a RedisError
Copy link
Member

Choose a reason for hiding this comment

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

This is fine, if Redis is having issues then there are fundamental issues with the environment that need to be resolved.

@github-project-automation github-project-automation bot moved this from 🆕 Untriaged to 🛑 Changes requested in Pull Request Overview Apr 25, 2025
@insertish insertish assigned insertish and unassigned insertish Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🛑 Changes requested
Development

Successfully merging this pull request may close these issues.

2 participants