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

Base_Engine: Fix issues initialising some Random objects #2984

Merged
merged 5 commits into from
Jan 9, 2023
Merged

Base_Engine: Fix issues initialising some Random objects #2984

merged 5 commits into from
Jan 9, 2023

Conversation

IsakNaslundBh
Copy link
Contributor

@IsakNaslundBh IsakNaslundBh commented Jan 9, 2023 •
edited by alelom
Loading

Issues addressed by this PR

Closes #2983

Closes #2962

  • Fully populate immutable objects
  • Try instantiating Fragments

Test files

https://burohappold.sharepoint.com/:f:/s/BHoM/EqZaPA5q69xGjXoY1xOrVMYB22zHLHgksKnhNYF5drTCog?e=n1tgyg

Changelog

Additional comments

@IsakNaslundBh IsakNaslundBh added the type:bug Error or unexpected behaviour label Jan 9, 2023
@IsakNaslundBh IsakNaslundBh requested a review from alelom January 9, 2023 09:57
@IsakNaslundBh IsakNaslundBh self-assigned this Jan 9, 2023
@IsakNaslundBh IsakNaslundBh requested a review from rwemay as a code owner January 9, 2023 10:02
@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@IsakNaslundBh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@IsakNaslundBh sorry, I didn't understand.
Was that comment an instruction for me? If so, could you state again what check you would like me to do?
For a list of available instructions, please see this wiki page.

alelom
alelom requested changes Jan 9, 2023
View reviewed changes
Copy link
Member

@alelom alelom left a comment

Choose a reason for hiding this comment

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

Tested, works great. Code makes sense too. We need to put in the documentation for compliance though.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check documentation compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@IsakNaslundBh sorry, I didn't understand.
Was that comment an instruction for me? If so, could you state again what check you would like me to do?
For a list of available instructions, please see this wiki page.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check documentation-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@IsakNaslundBh to confirm, the following actions are now queued:

  • check documentation-compliance

There are 9 requests in the queue ahead of you.

@alelom alelom self-requested a review January 9, 2023 10:26
alelom
alelom previously approved these changes Jan 9, 2023
View reviewed changes
Copy link
Member

@alelom alelom left a comment

Choose a reason for hiding this comment

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

Approving (see my previous comment for details)

@alelom
Copy link
Member

alelom commented Jan 9, 2023

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@alelom to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

The check documentation-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check copyright-compliance
@BHoMBot check dataset-compliance
@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@IsakNaslundBh to confirm, the following actions are now queued:

  • check copyright-compliance
  • check dataset-compliance
  • check unit-tests

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@IsakNaslundBh to confirm, the following actions are now queued:

  • check ready-to-merge

There are 4 requests in the queue ahead of you.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@IsakNaslundBh to confirm, the following actions are now queued:

  • check unit-tests

@IsakNaslundBh
Fix strings being generated in a more controlled fashion
68bcf13 
Using the same Random as is being used for other purposes
@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@IsakNaslundBh to confirm, the following actions are now queued:

  • check unit-tests

There are 15 requests in the queue ahead of you.

@alelom alelom self-requested a review January 9, 2023 12:09
alelom
alelom approved these changes Jan 9, 2023
View reviewed changes
Copy link
Member

@alelom alelom left a comment

Choose a reason for hiding this comment

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

Tested again after latest changes. Works well, code changes LGTM.

@alelom
Copy link
Member

alelom commented Jan 9, 2023

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@alelom to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

@alelom
Copy link
Member

alelom commented Jan 9, 2023

@BHoMBot check copyright-compliance
@BHoMBot check dataset-compliance
@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@alelom to confirm, the following actions are now queued:

  • check copyright-compliance
  • check dataset-compliance
  • check unit-tests

There are 11 requests in the queue ahead of you.

@alelom
Copy link
Member

alelom commented Jan 9, 2023

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@alelom to confirm, the following actions are now queued:

  • check ready-to-merge

There are 15 requests in the queue ahead of you.

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

The check unit-tests has already been run previously and recorded as a successful check. This check has not been run again at this time.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Jan 9, 2023

@FraserGreenroyd to confirm, the following actions are now queued:

  • check ready-to-merge

@FraserGreenroyd FraserGreenroyd merged commit 963a9a7 into develop Jan 9, 2023
@FraserGreenroyd FraserGreenroyd deleted the Base_Engine-#2983-FixIssuesWithProeprlyInitialisingRandomObjects branch January 9, 2023 12:40
@bhombot-ci bhombot-ci bot mentioned this pull request Mar 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alelom alelom alelom approved these changes

@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

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

Assignees

@IsakNaslundBh IsakNaslundBh

Labels
type:bug Error or unexpected behaviour
Projects
None yet
Milestone
No milestone
3 participants
@IsakNaslundBh @alelom @FraserGreenroyd