search

BHoM / BHoM_Engine

Internal manipulation of the BHoM
GNU Lesser General Public License v3.0
26 stars 13 forks source link

Serialiser_Engine: fix for MessageForDeleted to be picked up for properties of types that have multiple matching types #3371

Closed pawelbaran closed 2 months ago

pawelbaran commented 2 months ago

Issues addressed by this PR

Closes #3370

Test files

Changelog

Additional comments

pawelbaran commented 2 months ago

@BHoMBot check serialisation @BHoMBot check versioning

bhombot-ci[bot] commented 2 months ago
@pawelbaran to confirm, the following actions are now queued: - check `serialisation` - check `versioning`
pawelbaran commented 2 months ago

@BHoMBot check required

bhombot-ci[bot] commented 2 months ago
@pawelbaran 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[bot] commented 2 months ago
The check `serialisation` has already been run previously and recorded as a successful check. This check has not been run again at this time.
bhombot-ci[bot] commented 2 months ago
The check `versioning` has already been run previously and recorded as a successful check. This check has not been run again at this time.
pawelbaran commented 2 months ago

Thanks @adecler for the review, I have now addressed the comment 👍

pawelbaran commented 2 months ago

@BHoMBot check required

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

@BHoMBot check code-compliance @BHoMBot check copyright-compliance @BHoMBot check dataset-compliance @BHoMBot check unit-tests @BHoMBot check ready-to-merge

bhombot-ci[bot] commented 2 months ago
@pawelbaran to confirm, the following actions are now queued: - check `code-compliance` - check `copyright-compliance` - check `dataset-compliance` - check `unit-tests` - check `ready-to-merge`
pawelbaran commented 2 months ago

As a side note, the if-else sequence is a bit weird here.

Yeah I noticed that too and it tempted me a lot to refactor. I forced myself not to, mainly to avoid scope creep and extra review/testing resource to close out this PR.

pawelbaran commented 2 months ago

@BHoMBot check unit-tests

bhombot-ci[bot] commented 2 months ago
@pawelbaran to confirm, the following actions are now queued: - check `unit-tests` There are 1 requests in the queue ahead of you.
bhombot-ci[bot] commented 2 months ago
FAO: @FraserGreenroyd @pawelbaran is seeking dispensation on this Pull Request to skip a required check. Please can you provide authorisation for the check to be skipped, or provide assistance as appropriate. The check they wish to have dispensation on is unit-tests. If you are providing dispensation on this occasion, please reply with: > @BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. `27324447978`
pawelbaran commented 2 months ago

I will dispensate the failing unit test check on the basis of the fact that it has been failing consistently for a longer period of time, as can be seen here or here - each time the failure message is exactly the same.

pawelbaran commented 2 months ago

@BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. 27324447978

bhombot-ci[bot] commented 2 months ago
@pawelbaran I have now provided a passing check on reference `27324447978` as requested.