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

Slightly refactor code to make it look fresh #639

Merged
merged 3 commits into from
Dec 29, 2020

Conversation

zvirja
Copy link
Contributor

@zvirja zvirja commented Dec 22, 2020

In this PR I did a bit of polishing to make our code look nicer, cleaner and easier to read and maintain. Here is an approximate list of changes:

  • Consume the latest language features to make code look fresh and more concise
  • Convert some of the methods to lambdas. I made case-by-case decisions to ensure code looks better than before.
  • Remove ArgumentNullException checks from the internal parts of the code (i.e. mostly NSubstitute.Core), as now we have nullability in place to verify code is right. Code was not covered entirely anyway, so should not be an issue. If we have advanced clients who are using advanced API and push null values - well, either enable nullability or just be a nice person then 😊
  • Arrange missing visibility modifiers for consistency.
  • Did a bit of local refactorings to make code look nicer.
  • [Done via a separate commit] Split SubstituteExtensions into multiple partial classes, so that it's easier to navigate and maintain. Before it was just one huge junk with everything there.
  • [Done via a separate commit] Add a few R# configurations, as today we use either R# or Rider widely, so I wanted our code to look consistent.

There might other changes apart from the mentioned. Please review them if you don't like particular changes, so we could discuss those. Also I could completely revert/descope the individual commits if you would like to review/discuss those separately!

Thank you for your time!

@zvirja zvirja requested a review from dtchepak December 22, 2020 00:04
Copy link
Member

@dtchepak dtchepak left a comment

Choose a reason for hiding this comment

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

LGTM! 👍 🎉

@zvirja zvirja merged commit a4e2e6d into nsubstitute:master Dec 29, 2020
@zvirja zvirja deleted the consume-latest-language-features branch December 29, 2020 00:03
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.

2 participants