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.

Sign up for GitHub

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

Jump to bottom

Diffing_Engine: clarification of concept of Hash vs Diff; leverage persistentId #2105

Merged
merged 32 commits into from
Dec 2, 2020
Merged

Diffing_Engine: clarification of concept of Hash vs Diff; leverage persistentId #2105

merged 32 commits into from
Dec 2, 2020

Conversation

alelom
Copy link
Member

@alelom alelom commented Oct 27, 2020 •
edited
Loading

NOTE: Depends on

BHoM/BHoM#1084

Issues addressed by this PR

Closes #2095
Closes #2139

Test files

Changelog

Additional comments

@alelom alelom mentioned this pull request Oct 27, 2020
Closed
@alelom alelom self-assigned this Oct 27, 2020
@alelom alelom added the type:feature New capability or enhancement label Oct 27, 2020
@alelom alelom mentioned this pull request Nov 12, 2020
Merged
@alelom alelom marked this pull request as ready for review November 13, 2020 16:17
@alelom alelom changed the title Diffing_Engine: leverage persistentId Diffing_Engine: clarification of concept of Hash vs Diff; leverage persistentId Nov 13, 2020
@alelom
Align to minor changes in oM; HashComparer generic again
a7ed1f8
@alelom
Align to ComparisonConfig rename from DistinctConfig. Fixes
235b4f8 
Fixing CombineDiffs (adding null checks);
Fixing Hash(): failing to pick cloned object with HashFragment removed. Also added comments and tidy up for clarity.
@alelom
Update SetHashFragment.cs
2409049
@alelom
Copy link
Member Author

alelom commented Nov 16, 2020

@BHoMBot check core

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 16, 2020

@alelom to confirm, check-core task is now queued.

@alelom alelom mentioned this pull request Nov 16, 2020
Open
@alelom alelom mentioned this pull request Nov 16, 2020
Merged
@alelom
Copy link
Member Author

alelom commented Nov 27, 2020 •
edited
Loading

Regarding my comment above #2105 (comment)

I've raised an issue: #2193
so this can be done separately.

@alelom
Copy link
Member Author

alelom commented Dec 1, 2020

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Dec 1, 2020

@alelom to confirm, check-code-compliance, check-documentation-compliance, check-project-compliance, check-branch-compliance, check-dataset-compliance, and, if applicable, check-copyright-compliance tasks are now queued.

@bhombot-ci
Copy link

bhombot-ci bot commented Dec 1, 2020

Please be advised that the check with reference 1479287604 has more than 50 annotations of notes. API limitations restrict annotations to 50. You may need to rerun this check to obtain the next set when you make changes. At the time of reporting this check, there are 22 additional annotations waiting, made up of 0 errors and 22 warnings.

@bhombot-ci
Copy link

bhombot-ci bot commented Dec 1, 2020

@alelom just to let you know, I have provided a check-installer result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @FraserGreenroyd on BHoM

@alelom
Copy link
Member Author

alelom commented Dec 1, 2020

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Dec 1, 2020

@alelom to confirm, check-code-compliance, check-documentation-compliance, check-project-compliance, check-branch-compliance, check-dataset-compliance, and, if applicable, check-copyright-compliance tasks are now queued.

@bhombot-ci
Copy link

bhombot-ci bot commented Dec 1, 2020

Please be advised that the check with reference 1479798560 has more than 50 annotations of notes. API limitations restrict annotations to 50. You may need to rerun this check to obtain the next set when you make changes. At the time of reporting this check, there are 28 additional annotations waiting, made up of 0 errors and 28 warnings.

This was referenced Dec 1, 2020
Closed
Merged
@alelom alelom mentioned this pull request Dec 1, 2020
Closed
@alelom
Copy link
Member Author

alelom commented Dec 1, 2020

@BHoMBot check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Dec 1, 2020

@alelom to confirm, check-installer task is now queued.

@bhombot-ci
Copy link

bhombot-ci bot commented Dec 1, 2020

@alelom just to let you know, I have provided a check-installer result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @FraserGreenroyd on BHoM

@FraserGreenroyd
Copy link
Contributor

/azp run BHoM_Engine.CheckInstaller

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

FraserGreenroyd
FraserGreenroyd approved these changes Dec 1, 2020
View reviewed changes
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

I'm happy with what I've seen, I would hope others are too before merge 😄

@alelom
Copy link
Member Author

alelom commented Dec 2, 2020

@BHoMBot check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Dec 2, 2020

@alelom to confirm, check-installer task is now queued.

@BHoM BHoM deleted a comment from bhombot-ci bot Dec 2, 2020
@FraserGreenroyd FraserGreenroyd merged commit 70013ad into master Dec 2, 2020
@FraserGreenroyd FraserGreenroyd deleted the Diffing_Engine-#2095-leveragePersistentId branch December 2, 2020 16:41
@IsakNaslundBh IsakNaslundBh mentioned this pull request Dec 3, 2020
Closed
@alelom alelom mentioned this pull request Mar 3, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FraserGreenroyd FraserGreenroyd FraserGreenroyd approved these changes

@rolyhudson rolyhudson rolyhudson left review comments

@adecler adecler Awaiting requested review from adecler adecler is a code owner

@al-fisher al-fisher Awaiting requested review from al-fisher al-fisher is a code owner

@IsakNaslundBh IsakNaslundBh Awaiting requested review from IsakNaslundBh IsakNaslundBh is a code owner

@pawelbaran pawelbaran Awaiting requested review from pawelbaran pawelbaran is a code owner

@rwemay rwemay Awaiting requested review from rwemay rwemay is a code owner

@tg359 tg359 Awaiting requested review from tg359 tg359 is a code owner

Assignees

@alelom alelom

Labels
type:feature New capability or enhancement
Projects
None yet
Milestone
No milestone
3 participants
@alelom @FraserGreenroyd @rolyhudson