-
Notifications
You must be signed in to change notification settings - Fork 13
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
BHoM_Engine: add Query.Hash()
extension method
#2090
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor things but otherwise looks good.
I wonder if it wouldn't make more sense to have the Hash
method that takes all the arguments to be in Query
though. It kind of feels weird that one is in Query and one is in Compute
since the only difference is their number of arguments.
Looks good @alelom - added a small suggested change to one of the descriptions to distinguish |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed @alelom - unexpected behaviour between the two hash methods returning different results for same input
@al-fisher thanks - fixed in 6ba0075 |
Perfect - this is working as expected now! Thanks |
Co-authored-by: Al Fisher <[email protected]>
Isn't it a bit problematic that a call to |
Co-authored-by: Al Fisher <[email protected]>
Thanks for mopping up those descriptions @alelom 😄 |
/azp run BHoM_Engine.CheckCore |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run BHoM_Engine.CheckInstaller |
Azure Pipelines successfully started running 1 pipeline(s). |
@alelom , sorry I was just bothered by this: But that's not a big deal at all so that's fine. |
Issues addressed by this PR
Closes #2060
Test files