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

Fix for 'cargo test' #114

Open
wants to merge 3 commits into
base: freeze-feat-andreas
Choose a base branch
from
Open

Conversation

id3v1669
Copy link

@id3v1669 id3v1669 commented Apr 6, 2024

pr to fix issue #113

@Decodetalkers
Copy link
Collaborator

to freeze-feat-andreas branch, that is the developing branch

@id3v1669 id3v1669 mentioned this pull request Apr 6, 2024
13 tasks
@id3v1669 id3v1669 changed the base branch from main to freeze-feat-andreas April 6, 2024 11:13
id3v1669

This comment was marked as duplicate.

@Decodetalkers
Copy link
Collaborator

can you rebase it? thanks

@id3v1669
Copy link
Author

id3v1669 commented Apr 6, 2024

can you rebase it? thanks

I'm working on a new commit as in branch freeze-feat-andreas a lot more errors during cargo test. I'll ping you when done.

@id3v1669
Copy link
Author

id3v1669 commented Apr 6, 2024

@Decodetalkers Done.
Added also ignore to pixel-art in region.rs as it was also handled as error by cargo test
Bumped flake.lock due to error:

error: package clap_derive v4.5.3 cannot be built because it requires rustc 1.74 or newer, while the currently active rustc version is 1.70.0

@Decodetalkers
Copy link
Collaborator

@Decodetalkers Done. Added also ignore to pixel-art in region.rs as it was also handled as error by cargo test Bumped flake.lock due to error:

error: package clap_derive v4.5.3 cannot be built because it requires rustc 1.74 or newer, while the currently active rustc version is 1.70.0

maybe you can update nix, to which has higher version of rust

@id3v1669
Copy link
Author

id3v1669 commented Apr 6, 2024

maybe you can update nix, to which has higher version of rust

That's why I bumped flake.lock file, so newer version of cargo is used now cargo 1.76.0.

@Shinyzenith
Copy link
Member

I think this is a good time to run cargo test during builds even though tests don't exist. This helps us avoid such issues further down the line and we do have a WIP pr for integration testing in the future. How do you feel about this change? @id3v1669

@id3v1669
Copy link
Author

id3v1669 commented Apr 6, 2024

I think this is a good time to run cargo test during builds even though tests don't exist. This helps us avoid such issues further down the line and we do have a WIP pr for integration testing in the future. How do you feel about this change? @id3v1669

Sounds great, should I add cargo test into Makefile in this pr or you will do that later in pr for integration testing?

@Shinyzenith
Copy link
Member

Feel free to add it to this pr itself. Since it's a fairly minor change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants