Skip to content

Conversation

dcyonce
Copy link

@dcyonce dcyonce commented Jul 23, 2025

Added parameterless constructor
Added CompareTo(Guid)
Added Equals(Guid)
Added == and != overloads

Description

Motivation and Context

How Has This Been Tested?

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dependencies (update dependencies and changes associated, has no impact on code or features)
  • Unit Tests (add new Unit Test(s) or improved existing one(s), has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist:

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).
  • I have added new tests to cover my changes.

Added parameterless constructor
Added CompareTo(Guid)
Added Equals(Guid)
Added == and != overloads
Copy link

coderabbitai bot commented Jul 23, 2025

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (3)
  • Tests/NFUnitTestTypes/NFUnitTestTypes.nfproj is excluded by none and included by none
  • Tests/NFUnitTestTypes/UnitTestGuid.cs is excluded by none and included by none
  • nanoFramework.CoreLibrary/System/Guid.cs is excluded by none and included by none

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@dcyonce
Copy link
Author

dcyonce commented Jul 23, 2025 via email

@dcyonce
Copy link
Author

dcyonce commented Jul 23, 2025 via email

Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

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

Thank for this PR.
Mostly code style changes.
And if you could add a few unit tests to cover these new operators and catch the issues you've originally faced, that would be stellar!

Copy link
Author

@dcyonce dcyonce left a comment

Choose a reason for hiding this comment

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

I've made the changes... but I'm not sure how to use this GitHub Pull/Push Change process.
Do I submit a new Pull Request ?

@josesimoes
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

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

Looking good. Thanks.
Just added a few more.

You'll add the Unit tests in the next commit?

As for the pipelines, don't worry, for security reasons it requires a comment from a maintainer to run. I already did it for this last commit.

@dcyonce
Copy link
Author

dcyonce commented Aug 15, 2025

Sorry, I would add tests if I could, but I do not understand your test structure and how to run them.
I have added a file called UnitTestGuid.cs, but cannot figure out to run it. I always get deployment errors when trying to deploy NFUnitTestTypes

@josesimoes josesimoes changed the title Fixed null references in multiple methods, added == and != overloads Fixed null references and added == and != overloads in Guid Aug 18, 2025
@josesimoes
Copy link
Member

@dcyonce there are already a bunch of unit test for Guid. Please check here: Tests\NFUnitTestSystemLib\UnitTestGuid.cs

As for running the unit tests project: you have to build the solution (as usual). Following which, open the Test Explorer in VS.
You can run just the NFUnitTEstSystemLib group. Select it and hit the green arrow at the right.

image

The test runner should be able to install the virtual nanoclr (if not already there) and run the test group.
You can, of course, choose to run only the sub-group for Guid, but will take the same amount of time as the runner has to execute the complete group.

@dcyonce
Copy link
Author

dcyonce commented Aug 18, 2025 via email

@josesimoes
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@josesimoes
Copy link
Member

Doesn't seem to work. See the right panel: Outcomes: "351 Not Run"

That can show on test failure... I can't see any image, not sure if you've sent it.
Anyways, I've requested the pipeline to run the tests you've added. Let's how that goes.

@josesimoes
Copy link
Member

Unit tests failed because of the (expected) change in mscorlib assembly signature. This is caused because of the new operators you've added. Let me port these to the interpreter and see how this works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot compare Guid to Guid.Empty
3 participants