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

ci: bump pinned Rust to 1.70 #967

Merged
merged 16 commits into from
Jun 7, 2023
Merged

ci: bump pinned Rust to 1.70 #967

merged 16 commits into from
Jun 7, 2023

Conversation

iamwacko
Copy link
Contributor

@iamwacko iamwacko commented Jun 2, 2023

Description of change

Closes #964

@iamwacko
Copy link
Contributor Author

iamwacko commented Jun 2, 2023

cimg/rust hasn't been bumped to 1.70 yet. Would it be better to wait for the bump or switch images?

@AliSajid
Copy link
Contributor

AliSajid commented Jun 2, 2023

cimg-rust#110 Has that image coming up. Could be a moot point.

@iamwacko
Copy link
Contributor Author

iamwacko commented Jun 3, 2023

I made a whole bunch of commits thinking changes I had made were causing the rocket tests to fail, but now that I took a look at the tests in main I see that all that effort was for nothing. That also means that this should be ready to merge.

Copy link
Contributor

@oddgrd oddgrd left a comment

Choose a reason for hiding this comment

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

Apologies again for the CI confusion!

I left a comment about the clippy lint, I think we should rather refactor it. If we do ignore a clippy lint we should leave a comment about why as well. Regarding the once_cell that's fine, we can do it in a separate PR/later when the LazyCell equivalent is stabilized.

deployer/src/deployment/queue.rs Outdated Show resolved Hide resolved
@iamwacko
Copy link
Contributor Author

iamwacko commented Jun 6, 2023

#821 strikes again, with a side of errors of unknown cause. I'm getting the error below for all of deployment::deploy_layer::tests.

thread 'thread 'deployment::deploy_layer::tests::deployment_main_panicdeployment::deploy_layer::tests::deployment_bind_panic' panicked at '' panicked at 'states should go into 'Crashed' when panicking in main: [

@oddgrd oddgrd self-requested a review June 6, 2023 16:50
@oddgrd oddgrd added the XS label Jun 7, 2023
Copy link
Contributor

@oddgrd oddgrd left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this @iamwacko! I believe the deployer tests are spurious and not related to this PR.

Copy link
Contributor

@iulianbarbu iulianbarbu left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @iamwacko !

deployer/src/deployment/queue.rs Outdated Show resolved Hide resolved
@@ -20,7 +20,6 @@ http-body = { version = "0.4.5", optional = true }
http-serde = { version = "1.1.2", optional = true }
hyper = { workspace = true, optional = true }
jsonwebtoken = { workspace = true, optional = true }
once_cell = { workspace = true, optional = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: running a cargo sort might put the dep here again.

@iulianbarbu iulianbarbu merged commit 316b7a3 into shuttle-hq:main Jun 7, 2023
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.

[Improvement]: bump Shuttle's pinned rust version to 1.70
5 participants