Skip to content

Conversation

@Saibamen
Copy link
Contributor

No description provided.

Code formatting, remove unused usings
@PeterKottas
Copy link
Owner

Once again same as on you previous PR. White I appreciate the work, not sure yet how I feel about it. Pasting the response here for completeness sake.

This has a lot of very opinionated changes in it. Frankly I am not exactly sure how I feel about merging it. Mainly, I don't use resharper so chances are I'll be changing everything again when I autoformat. Thanks for the effort but I don't see real value in it right now. Maybe if resharper was free, I could adapt to it. But this obviously open source project so I can't expect everybody to have it installed and conform to it's autoformat rules.

@ka4ep
Copy link

ka4ep commented Oct 17, 2018

Just my five cents on ReSharper use here. A while ago I've spent some time on doing pretty much the same - adding [NotNull] or [CanBeNull] everywhere in this project code clone. ReSharper has found a dozen places where NRE are quite possible. My guess would be to take into account these warnings and put additional null checks in the original code. And remove ReSharper references and attributes. So that we know that it's almost null-safe. Just a sort of static NRE check. There also were a number of places where fields could be made readonly etc.

@PeterKottas
Copy link
Owner

Yeah that sounds good to me. Would be happy to accept PR that adds some confidence to how this lib works. It would of course make sense to add unit tests as well. But that would again have to be a contribution.

@Saibamen
Copy link
Contributor Author

@MohammadAsadi1372
Copy link

@MohammadAsadi1372
Copy link

Yeah that sounds good to me. Would be happy to accept PR that adds some confidence to how this lib works. It would of course make sense to add unit tests as well. But that would again have to be a contribution.

https://github.com/MohammadAsadi1372/MohammadAsadi.MasterService

@MohammadAsadi1372
Copy link

@MohammadAsadi1372
Copy link

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.

4 participants