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

add linting options #214

Merged
merged 4 commits into from
Sep 27, 2024
Merged

add linting options #214

merged 4 commits into from
Sep 27, 2024

Conversation

bircni
Copy link
Contributor

@bircni bircni commented Sep 23, 2024

I enabled some lints in clippy to improve the style of the code.
There are some todos open with missing panic docs.
It is not that good to use unwraps as we don't want to crash the users experience, maybe instead use the crate anyhow to safely handle errors.

Copy link
Owner

@blitzarx1 blitzarx1 left a comment

Choose a reason for hiding this comment

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

Thanks for such thorough work.

About using unwraps - totally agree. But I think we will come to this after the Layout feature is delivered and we start to stabilize APIs approaching to stable release. I will consider anyhow for sure.

What do you think about enhancing linter in the .github/workflows linter step?

@blitzarx1 blitzarx1 added the enhancement New feature or request label Sep 24, 2024
@bircni
Copy link
Contributor Author

bircni commented Sep 24, 2024

now also enhanced the workflow

@bircni
Copy link
Contributor Author

bircni commented Sep 24, 2024

still working on another suggestion - maybe I can finish that later on!

@bircni
Copy link
Contributor Author

bircni commented Sep 24, 2024

@blitzarx1 I started adding anyhow - maybe you could have a look at it?

@blitzarx1
Copy link
Owner

blitzarx1 commented Sep 24, 2024

lets extract anyhow changes to separate PR.

Also I am not sure this is the right time to add anyhow usage, while I am mid adding Layout feature. Let us return to anyhow after the Layout when we will be on stabilization track.

@bircni I am ready to continue wiht this PR, but without anyhow changes.

@bircni
Copy link
Contributor Author

bircni commented Sep 24, 2024

was just a check gonna revert them and finish the pr to make it ready for review finally!

@bircni bircni requested a review from blitzarx1 September 24, 2024 13:24
Copy link
Owner

@blitzarx1 blitzarx1 left a comment

Choose a reason for hiding this comment

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

Good begining for the stabilization track

@blitzarx1 blitzarx1 merged commit 77a7eaa into blitzarx1:master Sep 27, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants