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.

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

Fix Issue with Pushing Levels by using PriorityTypes #480

Merged
merged 6 commits into from
Nov 13, 2024

Conversation

@GCRA101 GCRA101 added the type:bug Error or unexpected behaviour label Oct 2, 2024
@GCRA101 GCRA101 added this to the BHoM 7.4 β MVP milestone Oct 2, 2024
@GCRA101 GCRA101 self-assigned this Oct 2, 2024
@GCRA101 GCRA101 added the type:feature New capability or enhancement label Oct 2, 2024
Copy link
Contributor

@IsakNaslundBh IsakNaslundBh left a comment

Choose a reason for hiding this comment

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

TO align with suggested changes in the BHoM_Adapter

Etabs_Adapter/Types/PriorityTypes.cs Outdated Show resolved Hide resolved
Co-authored-by: Isak Näslund <isak.naslund@burohappold.com>
@IsakNaslundBh IsakNaslundBh dismissed their stale review October 18, 2024 06:59

Requested changes now addressed. Dismissing my request for change review

@IsakNaslundBh
Copy link
Contributor

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?

@peterjamesnugent
Copy link
Member

@HugoVanLooveren to complete a functionality review.

@peterjamesnugent peterjamesnugent removed the request for review from Chrisshort92 November 7, 2024 14:11
Copy link

@HugoVanLooveren HugoVanLooveren left a comment

Choose a reason for hiding this comment

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

Tested with ETABS 21 & 19
Used BHoM Version 7.2.1.0

With 19 it all worked (opened clean ETABS file, as the example file was for 20)

With 21, the levels got pushed, however, only 1 bar got pushed.
Got this error:

  1. This component failed to run properly.
  • Error: Unable to cast object of type 'System.Boolean' to type 'System.Double[]'.
  • Occured in BH.UI.Base.Caller.Run(List`1 inputs)
    called from BH.UI.Base.Caller.Run()
  • Are you sure you have the correct type of inputs? Check their description for more details.

@HugoVanLooveren
Copy link

@GCRA101 & @peterjamesnugent
Just had a look and ETASB 21 did not work.
Dunno if that should become a new issue or something though, as the levels got pushed properly.

Copy link

bhombot-ci bot commented Nov 8, 2024

@GCRA101 just to let you know, I have provided a check-versioning 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 BHoM_Adapter

Copy link

bhombot-ci bot commented Nov 8, 2024

@GCRA101 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 @GCRA101 on BHoM_Adapter

@GCRA101
Copy link
Contributor Author

GCRA101 commented Nov 8, 2024

@BHoMBot check required

Copy link

bhombot-ci bot commented Nov 8, 2024

@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
Copy link
Contributor Author

GCRA101 commented Nov 8, 2024

@BHoMBot check copyright-compliance

Copy link

bhombot-ci bot commented Nov 8, 2024

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

  • check copyright-compliance

There are 2 requests in the queue ahead of you.

Copy link

bhombot-ci bot commented Nov 8, 2024

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

Copy link

bhombot-ci bot commented Nov 8, 2024

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

@peterjamesnugent
Copy link
Member

@GCRA101 & @peterjamesnugent Just had a look and ETASB 21 did not work. Dunno if that should become a new issue or something though, as the levels got pushed properly.

ETABS21 is being covered in a seperate PR:

Copy link
Member

@peterjamesnugent peterjamesnugent left a comment

Choose a reason for hiding this comment

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

Approving based on @HugoVanLooveren's review, and the issue with Etabs21 being covered in a seperate PR:
#483

@GCRA101
Copy link
Contributor Author

GCRA101 commented Nov 11, 2024

@BHoMBot check ready-to-merge

Copy link

bhombot-ci bot commented Nov 11, 2024

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

  • check ready-to-merge

@peterjamesnugent peterjamesnugent merged commit 5ad3b80 into develop Nov 13, 2024
10 checks passed
@peterjamesnugent peterjamesnugent deleted the BHoM_Adapter-#390-AddPriorityTypesForPush branch November 13, 2024 17:53
@BHoMBot BHoMBot mentioned this pull request Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error or unexpected behaviour type:feature New capability or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Levels are not pushing to a new model and an error is incorrectly raised
4 participants