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(ci): fix make clippy script and clippy CI job #220

Merged
merged 18 commits into from
Jun 19, 2024
Merged

feat(ci): fix make clippy script and clippy CI job #220

merged 18 commits into from
Jun 19, 2024

Conversation

petarvujovic98
Copy link
Contributor

Configure the CI job to properly run clippy

@Brechtpd Brechtpd requested a review from dionysuzx June 8, 2024 09:27
@dionysuzx
Copy link
Contributor

@petarvujovic98 there is some merge conflicts now as the ci was refactored

@petarvujovic98
Copy link
Contributor Author

@d1onys1us @Brechtpd Given the CI passes, this should be ready for review

@petarvujovic98 petarvujovic98 requested a review from Brechtpd June 17, 2024 13:44
dionysuzx
dionysuzx reviewed Jun 18, 2024
View reviewed changes
Copy link
Contributor

@dionysuzx dionysuzx left a comment

Choose a reason for hiding this comment

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

LGTM, just one extra file, also there are some files that seem unrelated to CI / clippy; maybe that should be in a separate PR?

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@petarvujovic98
Copy link
Contributor Author

@d1onys1us The rust files with the changes are actual fixes that cause CI to fail with the new clippy set up. They were not checked properly before.

@petarvujovic98 petarvujovic98 self-assigned this Jun 18, 2024
Brechtpd
Brechtpd reviewed Jun 18, 2024
View reviewed changes
Copy link
Contributor

@Brechtpd Brechtpd left a comment

Choose a reason for hiding this comment

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

Looks good! Just added a comment about clippy specific script.

script/clippy.sh Outdated Show resolved Hide resolved
@petarvujovic98 petarvujovic98 added this pull request to the merge queue Jun 19, 2024
Merged via the queue into main with commit 62158a0 Jun 19, 2024
22 checks passed
@petarvujovic98 petarvujovic98 deleted the clippy-ci branch June 19, 2024 09:04
@taiko-kitty taiko-kitty mentioned this pull request Jun 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dionysuzx dionysuzx dionysuzx left review comments

@Brechtpd Brechtpd Brechtpd approved these changes

Assignees

@petarvujovic98 petarvujovic98

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@petarvujovic98 @dionysuzx @Brechtpd