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

Bug fix/validation for parameters #92

Closed

Conversation

Kaybangz
Copy link

Which issue are you addressing?

#51

How have you addressed the issue?

  1. Removed Barrier() from Parallel.Ordered method
  2. Moved the body of the If statements in ValidateParameters() method to a new indented line
  3. In Exception.cs, added a space before the : tokens
  4. Used ValidateParameters() only at the top of the internal For handler
  5. Removed the spaces on line 250 in Parallel.cs
  6. Added a descriptive message to the custom NotInForException

@computablee computablee self-requested a review October 16, 2023 23:07
Copy link
Owner

@computablee computablee left a comment

Choose a reason for hiding this comment

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

The checks are failing, the code doesn't compile. Please fix the compile issues and we'll go from there.

@computablee computablee linked an issue Oct 16, 2023 that may be closed by this pull request
@Kaybangz
Copy link
Author

Hey @computablee, do you seem to have any idea why compilation is failing and how I can go about fixing?

I would also like to know how to run tests on my end, to ensure that everything works before making a PR.

I apologize for the inconvenience

@computablee
Copy link
Owner

It looks like you have mismatched names in your added function (e.g., you accept chunkSize as a parameter, but later reference a chunk_size variable... this is the case with numerous variables).

If you have the dotnet CLI installed, you can navigate to the DotMP-Tests/ directory and run dotnet test --verbose in your command line—this will ensure not only that everything compiles, but that some 40-ish other integration tests pass.

I also recommend reading the new CONTRIBUTING.md in the main branch I updated tonight—it outlines several important things about opening a PR, such as things that are grounds for immediate rejection, code formatting and styling, and testing. These will be enforced moving forward.

@computablee
Copy link
Owner

Also, I noticed that the linting check failed on the dotnet-format pass. This can be solved by navigating to the DotMP/ folder and running dotnet format. This tool will automatically resolve any formatting issues. Make sure to do that before you open the PR.

@Kaybangz
Copy link
Author

Alright, noted.

@Kaybangz
Copy link
Author

I keep getting this error Failed to read environment variables [DOTNET_STARTUP_HOOK], HRESULT: 0X800700CB when I try to run the dotnet test --verbose, dotnet format or dotnet build.

Can I push my changes without testing, since I have made all necessary changes or is there any other suggestion you have for rectifying this problem?

Build_error_1
Build_error_2
Build_error_3

@computablee
Copy link
Owner

Hey @Kaybangz, sorry for the late response. I have recently adopted a cat and have not even had time to open GitHub.

I would recommend asking around in the C# Discord to see if they can fix your problem. Unfortunately, I cannot help diagnose issues with the dotnet CLI—I merely maintain a .NET project which uses it.

@Kaybangz
Copy link
Author

Congrats on adopting a cat😊

I'll be sure to check out the Discord group

@computablee
Copy link
Owner

In order to expedite this issue to get the next major release out, I am closing this PR and self-assigning #51.

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.

Need more rigorous error checking
2 participants