BHoM / BHoM_Adapter

GNU Lesser General Public License v3.0
7 stars 7 forks source link

Add Priority Types For Push #393

Closed GCRA101 closed 1 week ago

GCRA101 commented 1 month ago

NOTE: Depends on

https://github.com/BHoM/ETABS_Toolkit/pull/480

Issues addressed by this PR

Closes #390

BHoM_Adapter now equipped with a new attribute PropertyTypes defining priority order for the push of objects into software packages in addition to DependencyTypes. This change has been required due to the requirements from software packages like ETABS to push objects following a specific hierarchy.

Test files

Video Demonstration - Issue https://burohappold.sharepoint.com/:v:/r/sites/BHoM/02_Current/12_Scripts/02_Pull%20Request/BHoM/ETABS_Toolkit/%23480-AddPriorityTypesForPush/Push%20Levels%20Issue.mp4?csf=1&web=1&e=7cEacd Video Demonstration - Solution https://burohappold.sharepoint.com/:v:/r/sites/BHoM/02_Current/12_Scripts/02_Pull%20Request/BHoM/ETABS_Toolkit/%23480-AddPriorityTypesForPush/Push%20Levels%20Sorted.mp4?csf=1&web=1&e=cDUbje Json File https://burohappold.sharepoint.com/sites/BHoM/_layouts/15/download.aspx?UniqueId=aa695dbddee44123b35aecc6843d658a&e=mvnMou Grasshopper File https://burohappold.sharepoint.com/:u:/r/sites/BHoM/02_Current/12_Scripts/02_Pull%20Request/BHoM/ETABS_Toolkit/%23480-AddPriorityTypesForPush/Test%20Script.gh?csf=1&web=1&e=Lzfzec Etabs File https://burohappold.sharepoint.com/:u:/r/sites/BHoM/02_Current/12_Scripts/02_Pull%20Request/BHoM/ETABS_Toolkit/%23480-AddPriorityTypesForPush/Test%20ETABS%20Model.EDB?csf=1&web=1&e=lj7JY7

Changelog

alelom commented 1 month ago

Had a look at this PR and the ETABS one. Great work @GCRA101 ! 🚀 I agree with the changes suggested by @IsakNaslundBh .

GCRA101 commented 1 month ago

@IsakNaslundBh, @alelom, thanks again for the comments. I've incorporated them in the latest commit 3587c9286f9999586a26d870c950f3860f3442fb

IsakNaslundBh commented 1 month ago

Happy with this from a code perspective now, and have dismissed my review. @Chrisshort92 Could you please assist in testing this from a functionality point of view?

GCRA101 commented 3 weeks ago

@peterjamesnugent , @HugoVanLooveren, rebuilt and rechecked the code after the modifications and it all works fine ;).

ETABS vs GH View Levels Push v2

peterjamesnugent commented 2 weeks ago

@BHoMBot check required @BHoMBot check copyright-compliance

bhombot-ci[bot] commented 2 weeks ago
@peterjamesnugent 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` - check `copyright-compliance` There are 3 requests in the queue ahead of you.
bhombot-ci[bot] commented 2 weeks ago
Please be advised that the check with reference 32650160492 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 14 additional annotations waiting, made up of 14 errors and 0 warnings.
peterjamesnugent commented 2 weeks ago

@GCRA101 for this one you need to sort out the copyright-compliance errors and potentially update from develop branch (let me know if you're unsure what to do).

GCRA101 commented 2 weeks ago

@BHoMBot check required

bhombot-ci[bot] commented 2 weeks ago
@GCRA101 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`
GCRA101 commented 2 weeks ago

@BHoMBot check copyright-compliance

bhombot-ci[bot] commented 2 weeks ago
@GCRA101 to confirm, the following actions are now queued: - check `copyright-compliance`
GCRA101 commented 2 weeks ago

@BHoMBot check ready-to-merge

bhombot-ci[bot] commented 2 weeks ago
@GCRA101 to confirm, the following actions are now queued: - check `ready-to-merge` There are 8 requests in the queue ahead of you.
bhombot-ci[bot] commented 1 week ago
@GCRA101 just to let you know, I have provided a `check-ready-to-merge` 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 @GCRA101 on ETABS_Toolkit