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

feat(model): Implement Premium Button Style #2363

Merged
merged 31 commits into from
Aug 31, 2024
Merged

feat(model): Implement Premium Button Style #2363

merged 31 commits into from
Aug 31, 2024

Conversation

rickmartensnl
Copy link
Contributor

This PR implements the new premium button style.

Refs:

@github-actions github-actions bot added c-model Affects the model crate c-util Affects the util crate c-validate Affects the validate crate t-feature Addition of a new feature labels Jul 18, 2024
@suneettipirneni suneettipirneni self-requested a review July 27, 2024 23:26
@rickmartensnl
Copy link
Contributor Author

rickmartensnl commented Jul 28, 2024 •
edited
Loading

CI did not complete since tokio-macros got updated 5 days ago to version 2.4.0, this version it requires rust version 1.70 or later. And with that update it also has updated tokio, which also requires rust version 1.70 or later.
When manually downgrading tokio to version 1.38.1, using cargo update -p tokio@1 --precise 1.38.1, I am able to run the cargo +1.67 check --all-features --workspace --exclude book command and it completes successfully on my Mac.

Currently I see 2 options to fix this issue:

  • Manually downgrade tokio, where used, to version 1.38.1. This keeps the minimum rust version of 1.67.
  • Update the minimum rust version to 1.70.

Please let me know which option you think is the best, then I'll fix it.
I am for now going with updating the rust version to 1.70, since the new release of twilight(1.60) is still a release candidate.

@github-actions github-actions bot added c-all Affects all crates or the project as a whole c-book Affects the book labels Jul 28, 2024
@vilgotf
Copy link
Member

vilgotf commented Jul 28, 2024 •
edited
Loading

We don't actually need to change our MSRV until we depend on tokio >1.38 with the new MSRV requirements. Could you instead add a new CI step pinning the offending package versions? cargo add "tokio@>=1.19.0,<1.39.0" -p twilight-gateway etc.

@github-actions github-actions bot added c-cache Affects the cache crate c-http Affects the http crate c-http-ratelimiting Affects the http ratelimiting crate labels Jul 28, 2024
@rickmartensnl
Revert fixing Clippy warnings.
09fbe18 
These Clippy warnings werent introduced by me, and @laralove143 requested me to revert them.
@rickmartensnl
Fix formatting imports.
644fd0d
@github-actions github-actions bot removed c-cache Affects the cache crate c-http-ratelimiting Affects the http ratelimiting crate c-http Affects the http crate labels Aug 3, 2024
@rickmartensnl
Revert more Clippy fixes.
d27c529 
These Clippy warnings werent introduced by me, and @laralove143 requested me to revert them.
@rickmartensnl
Fix imports to multiline.
3c9be6a
@rickmartensnl
Fix last imports.
3fcd693
@rickmartensnl
Copy link
Contributor Author

rickmartensnl commented Aug 4, 2024 •
edited
Loading

Okay I guess Clippy fails due to the revert of the Clippy fixes with the spacing, I'll redo the changes to fix the CI.

@rickmartensnl
Fix all Clippy warns.
5c97433 
This should fix every single Clippy warns that may cause CI failure.
@github-actions github-actions bot added c-cache Affects the cache crate c-http Affects the http crate c-http-ratelimiting Affects the http ratelimiting crate labels Aug 4, 2024
@vilgotf vilgotf mentioned this pull request Aug 10, 2024
Merged
@rickmartensnl
Copy link
Contributor Author

As far as I am concerned this PR is good to get merged.

laralove143
laralove143 requested changes Aug 27, 2024
View reviewed changes
Copy link
Member

@laralove143 laralove143 left a comment

Choose a reason for hiding this comment

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

A tiny documentation request

twilight-model/src/http/interaction.rs Outdated Show resolved Hide resolved
twilight-model/src/http/interaction.rs Outdated Show resolved Hide resolved
@laralove143
Copy link
Member

Okay I guess Clippy fails due to the revert of the Clippy fixes with the spacing, I'll redo the changes to fix the CI.

Sorry for the late reply/review

I'm asking this to the team because I'm not sure about what to do. This PR contains unrelated fixes of lints and formatting changes, but our CI seems to fail without these changes. Is this PR still good to merge?

@rickmartensnl
Documentation fixes.
4d50e42 
Thank you @laralove143 for the review.
laralove143
laralove143 approved these changes Aug 29, 2024
View reviewed changes
Copy link
Member

@laralove143 laralove143 left a comment

Choose a reason for hiding this comment

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

Thank you for your work.

@github-actions github-actions bot removed c-cache Affects the cache crate c-http Affects the http crate c-http-ratelimiting Affects the http ratelimiting crate labels Aug 31, 2024
@rickmartensnl
Copy link
Contributor Author

Merge issue resolved.

@suneettipirneni suneettipirneni enabled auto-merge (squash) August 31, 2024 19:23
@suneettipirneni suneettipirneni merged commit 5ae72bc into twilight-rs:main Aug 31, 2024
9 checks passed
@Gelbpunkt
Copy link
Member

That CI change should have been reverted with the recent MSRV bump to 1.79.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laralove143 laralove143 laralove143 approved these changes

@suneettipirneni suneettipirneni suneettipirneni approved these changes

@ImDarkDiamond ImDarkDiamond ImDarkDiamond approved these changes

@AEnterprise AEnterprise AEnterprise approved these changes

Assignees
No one assigned
Labels
c-all Affects all crates or the project as a whole c-model Affects the model crate c-util Affects the util crate c-validate Affects the validate crate t-ci Anything to do with CI. t-feature Addition of a new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@rickmartensnl @vilgotf @laralove143 @Gelbpunkt @AEnterprise @ImDarkDiamond @suneettipirneni