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

style: simplify some statements for readability #271

Merged

Conversation

hamirmahal
Copy link
Contributor

Closes #270

Copy link
Contributor Author

@hamirmahal hamirmahal left a comment

Choose a reason for hiding this comment

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

This shouldn't cause any user-facing changes.

The goal here is to improve code readability and maintainability.

@hamirmahal
Copy link
Contributor Author

dua-cli is an excellent command-line tool.

I like that development builds are really fast, especially relative to other Rust projects I've contributed to.

Would you want a check against build time regressions, to keep build times solid?

I can look into adding that to CI in a future pull request if you're interested.

Copy link
Owner

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot, much appreciated!

Would you want a check against build time regressions, to keep build times solid?

I think when it happens and you notice it, I would rather like to welcome a PR to fix the regression if it is fixable at all.

The reason I wouldn't want complexity to be added to CI for this is that I believe it would have to be timing based, and that will be flaky too easily.

Thanks for your understanding.

If you have more ideas on contributions you'd like to make, please feel free to do it with a PR right away - I really want to keep your overhead low and the extra issue here didn't seem necessary.

@Byron Byron merged commit 3bc25bd into Byron:main Dec 13, 2024
2 checks passed
@hamirmahal hamirmahal deleted the style/simplify-some-statements-for-readability branch December 13, 2024 22:11
@hamirmahal
Copy link
Contributor Author

You're welcome! We could add some leeway, maybe 10% of the average build time of a few runs, to circumvent some flakiness, but it's completely up to you.

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

Successfully merging this pull request may close these issues.

Some statements can be simplified for readability
2 participants