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

SD #8065

Closed
wants to merge 6 commits into from
Closed

SD #8065

wants to merge 6 commits into from

Conversation

kornkaobat
Copy link

@kornkaobat kornkaobat commented Mar 1, 2020

Nuked

@thewildtree
Copy link
Contributor

Instead of using GitHub's editor to blindly modify the code, I'd recommend you install an actual IDE to compile and test the code before you commit. You can also use the InspectCode.ps1 from the root directory of this repo to run the code quality checks locally instead of relying on CI.

@bdach
Copy link
Collaborator

bdach commented Mar 1, 2020

This PR is intensely confused. To list off in no particular order:

  • Editing code files through the github interface seems to have entirely mangled line endings (although I suspect it might have been due to copying from a local editor with different line endings), rendering the diff useless.

  • The code seems to have been developed by trial and error and "compile tested" on CI. This is unacceptable.

  • There is also added commented code copy-pasted from Stack Overflow, that in parts doesn't even make sense or isn't valid C#.

  • As for this claim:

    Code is fully working using .NET Core 2.0.0. I tried it with the latest version of .NET but it won't compile with the latest version.

    We're now on .NET Core 3.1, so this is also not acceptable.

I'm going to go ahead and close this. If you attempt this again, please install an actual IDE, test your changes and understand the programming language you're attempting to use.

@bdach bdach closed this Mar 1, 2020
@kornkaobat kornkaobat changed the title Add Standard Deviation Codes (Don't Merge, PR for code review) SD Mar 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants