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(FEEL): reported error switching #285

Merged
merged 1 commit into from
Sep 18, 2023
Merged

fix(FEEL): reported error switching #285

merged 1 commit into from
Sep 18, 2023

Conversation

smbea
Copy link
Contributor

@smbea smbea commented Sep 15, 2023

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Sep 15, 2023
@smbea smbea requested review from a team, philippfromme and barmac and removed request for a team September 15, 2023 14:05
@nikku
Copy link
Member

nikku commented Sep 18, 2023

Do you remember why we had local > global in the past? And can you make a good call for global > local? I'm happy to merge this, we just want to ensure we know why this is a good direction.

@smbea
Copy link
Contributor Author

smbea commented Sep 18, 2023

Do you remember why we had local > global in the past?

I don't really know. I guess it's so the user can get the most context: a syntax error is more specific than just "not valid"

And can you make a good call for global > local?

I vouch for this because you can still get the syntax error by hovering the highlighted area.

image

@nikku
Copy link
Member

nikku commented Sep 18, 2023

Thanks. Makes sense to me, along with prior discussions in camunda/camunda-modeler#3845 (comment) and following.

If there is a global (in the larger context) error provided, we'll always show that over the local error.

@nikku
Copy link
Member

nikku commented Sep 18, 2023

Thinking about it global should rather be called externally provided to make that clear. Not required for this fix though.

@smbea smbea merged commit 375838b into main Sep 18, 2023
9 of 11 checks passed
@smbea smbea deleted the fix-marker branch September 18, 2023 14:03
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants