BHoM / BHoM_UI

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

Update run to reset log suppression after running component #486

Closed FraserGreenroyd closed 6 months ago

FraserGreenroyd commented 6 months ago

NOTE: Depends on

https://github.com/BHoM/BHoM_Engine/pull/3307

Issues addressed by this PR

Fixes #485

Additional comments

You may be wondering why I've opted to query the suppression state to then reset it, however, the use case for this is if a user of visual programming has, for whatever reason, opted to suppress things themselves, we don't want to blindly reset the suppression after a method runs in case it failed to reset it itself.

The UX on that would be this:

This could also be the case for methods which don't crash:

Therefore, the way of doing it in this PR allows for this UX instead:

Now, we can debate whether a user should ever be suppressing their own errors or not as much as we like (and I would gladly welcome it), however, BHoM philosophy is that methods are reflected up at all times, which is the case for suppressing events as well, which means that, regardless of how we feel as to whether users should do that, they can do it and thus we need to support that UX.

Hopefully that all makes sense!

Edit to capture a conversation had with @IsakNaslundBh:

While this works well for the use cases above and the use case captured in #485 , this does now mean the following situation is possible:

It's a convoluted system however, @IsakNaslundBh and I have both agreed this is an acceptable risk at this time because:

FraserGreenroyd commented 6 months ago

@BHoMBot check compliance

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd 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` There are 12 requests in the queue ahead of you.
FraserGreenroyd commented 6 months ago

@BHoMBot check core

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `core` There are 12 requests in the queue ahead of you.
bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd 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 @FraserGreenroyd on BHoM_Engine
bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd 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 @FraserGreenroyd on BHoM_Engine
Tom-Kingstone commented 6 months ago

@BHoMBot check ready-to-merge

bhombot-ci[bot] commented 6 months ago
@Tom-Kingstone to confirm, the following actions are now queued: - check `ready-to-merge`
FraserGreenroyd commented 6 months ago

@BHoMBot check versioning @BHoMBot check installer @BHoMBot check compliance @BHoMBot check core

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `versioning` - check `installer` - check `code-compliance` - check `documentation-compliance` - check `project-compliance` - check `branch-compliance` - check `dataset-compliance` - check `copyright-compliance` - check `core`
Tom-Kingstone commented 6 months ago

@BHoMBot check ready-to-merge

bhombot-ci[bot] commented 6 months ago
@Tom-Kingstone to confirm, the following actions are now queued: - check `ready-to-merge`