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

Replace all clippy::unwrap_used with Results and ? in all tests. #5051

Closed
5 tasks done
dessalines opened this issue Sep 24, 2024 · 1 comment · Fixed by #5064
Closed
5 tasks done

Replace all clippy::unwrap_used with Results and ? in all tests. #5051

dessalines opened this issue Sep 24, 2024 · 1 comment · Fixed by #5064
Labels
area: maintenance enhancement New feature or request extra: good first issue Good for newcomers

Comments

@dessalines
Copy link
Member

Requirements

  • Is this a feature request? For questions or discussions use https://lemmy.ml/c/lemmy_support
  • Did you check to see if this issue already exists?
  • Is this only a feature request? Do not put multiple feature requests in one issue.
  • Is this a backend issue? Use the lemmy-ui repo for UI / frontend issues.
  • Do you agree to follow the rules in our Code of Conduct?

Is your proposal related to a problem?

In most of our tests, we use clippy::unwrap_used annotation, when these aren't necessary.

Describe the solution you'd like.

They can be replaced with -> LemmyResult<()> (or another result type), and use ? in place of unwrap

Describe alternatives you've considered.

NA

Additional context

#5048

@dessalines dessalines added enhancement New feature or request area: maintenance labels Sep 24, 2024
@Nutomic Nutomic added the extra: good first issue Good for newcomers label Sep 25, 2024
@Nutomic
Copy link
Member

Nutomic commented Sep 25, 2024

This is a good issue for new contributors to get started. You basically need to follow these steps:

  1. Setup local dev environment
  2. Search for clippy::unwrap_used in the code and remove it one at a time
  3. Run ./scripts/lint.sh and fix errors one at a time by replacing .unwrap() with ?, and changing function return type to LemmyResult<()>
  4. Commit frequently whenever it is compiling without errors
  5. Repeat from 2

Its not necessary to remove all occurences in a single pull request, doing only part of them is also helpful. There are some cases where unwrap is used for Option, these can probably remain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: maintenance enhancement New feature or request extra: good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants