Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore: Upgrade Rust version #193

Merged

Conversation

dragoljub-duric
Copy link
Contributor

@dragoljub-duric dragoljub-duric commented Feb 9, 2024

Problem:
We need rust >= 1.74 to use div_ceil().

Solution:
Upgrade the Rust version.

@dragoljub-duric dragoljub-duric requested a review from a team as a code owner February 9, 2024 15:21
@ielashi
Copy link
Contributor

ielashi commented Feb 12, 2024

@dragoljub-duric Clear communication is a very valuable skill to develop as an engineer - PR titles and descriptions are especially important components of that.

Reading the current PR title/description, I see:

  • Upgrade Rust in CI - this is misleading, since you're upgrading Rust everywhere and not only in CI.
  • There's no description, so I'm left wondering why you're making this upgrade. Is it just a casual upgrade, was there something blocking you that needed such an upgrade, some other reason?

I'll approve but I'd encourage you to enhance the title/description and take that into account in future PRs.

@dragoljub-duric dragoljub-duric changed the title chore: Upgrade Rust version in CI chore: Upgrade Rust version Feb 12, 2024
@dragoljub-duric dragoljub-duric merged commit 5ac483a into main Feb 12, 2024
9 checks passed
@dragoljub-duric dragoljub-duric deleted the EXC-1553-upgrade-rust-version-in-stable-structures-ci branch February 12, 2024 10:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants