-
-
Notifications
You must be signed in to change notification settings - Fork 182
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
Strengthen EditorConfig to help enforce coding standards #775
Conversation
Update code to match coding standards
Fix release version
What's the coding standard that we follow here? |
StyleCop + .editorconfig rules, it's the same set of rules that we run with the other ReactiveMarbles and ReactiveUI projects. The main alterations are ensuring that code is maintained in order of modifier and making suggestions for recommended changes. Errors are raised when there's code that doesn't meet the core requirements. |
Based on the dotnet runtime settings. Documented on the rxui website. |
Let's get outstanding PRs in before this, otherwise we'll get some merge conflicts. |
Reduce code bloat by using extension method for ArgumentNullException's. This was chosen due to current target frameworks. If in the future netstandard2.1 + compliant frameworks are used we can switch methods.
src\DynamicData.Tests\Cache\TransformFixtureParallel.cs line 95 - Test Needs fixing or updated Functionality needs checking
@JakenVeina src\DynamicData.Tests\Cache\TransformFixtureParallel.cs line 95 - Test Needs fixing or updated Functionality needs checking after PR #771 I have Skipped this test for the moment to ensure that there's no other tests with issues. |
Test - src\DynamicData.Tests\Cache\TransformFixtureParallel.cs SameKeyChanges on line 95 needs attention
Skip has been removed to invoke test failure - awaiting Test update or core functionality update. |
Running the same test on Main doesn't produce the same issue, so perhaps there is a merge issue somewhere. |
There's nothing wrong with the code, and nothing that any user has ever complained about in almost 10 years. The issue I think is purely down to mixing paradigms, and making tests work. Parallel.for sucks, but seemed a good idea at the time. Possibly in the future I'll change it to use pure Rx. I am happy for the test to be skipped. |
In which case, maybe my above comment is complete rubbish |
No problem, I think there must be something in the auto merge that happened, it's 05:43 here I better go back to sleep :) |
@ChrisPulman thanks for helping and have a good journey. I am in the air too tomorrow. |
So the issue is not coming from #771,
It looks like this issue was present on the branch before #771 was merged. Commit 6395e7d seems to be the source, rolling back to just before that commit eliminates the issue, and pulling just that commit back in reintroduces it. |
Thank you very much @JakenVeina |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Update code to match updated coding standard.
Produces errors when coding doesn't match styling and recommended C# rules.