-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add ViewPlan, Sheet, Viewport and Drawing Area methods #1313
Add ViewPlan, Sheet, Viewport and Drawing Area methods #1313
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.
In addition to the comments below, we should switch all BH.Engine.Base.Compute.RecordWarning
call to using the $
string interpolation syntax instead of string + string + string etc.
6421a50
to
364a620
Compare
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.
Last few remarks left 👍
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.
Last of the last from my side 👍
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.
Happy to approve from code side 👍 To be merged after functional verification.
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.
Still happy with the code 👍
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.
Re-approving on the basis of previous review and the scope of the recent commit - looking forward to versioning passing this time 👍
67a9f06
to
75e673a
Compare
@pawelbaran to confirm, the following actions are now queued:
There are 41 requests in the queue ahead of you. |
@pawelbaran to confirm, the following actions are now queued:
|
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.
Still happy with the changes after the last commit
@pawelbaran to confirm, the following actions are now queued:
|
@pawelbaran to confirm, the following actions are now queued:
There are 3 requests in the queue ahead of you. |
@BHoMBot check versioning |
@pawelbaran to confirm, the following actions are now queued:
|
Please be advised that the check with reference 11917505152 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 382 additional annotations waiting, made up of 382 errors and 0 warnings. |
@BHoMBot check versioning |
@pawelbaran to confirm, the following actions are now queued:
|
@BHoMBot check ready-to-merge |
@pawelbaran to confirm, the following actions are now queued:
|
NOTE: Depends on
Issues addressed by this PR
Closes #1314
Methods needed for generating views and sheets.
Test files
Changelog
Additional comments