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: Instance of with correct duration names #535

Merged
merged 2 commits into from
Oct 19, 2022
Merged

Conversation

sccalabr
Copy link
Contributor

@sccalabr sccalabr commented Oct 11, 2022

Description

  • instance of doesn't work for durations

Expected behavior

null instance of years and months duration -> false
null instance of days and time duration -> false

duration("P3M") instance of years and months duration -> true
duration("PT4H") instance of days and time duration -> true

Related issues

closes #155

@sccalabr sccalabr changed the title Issue 115 Issue 115 Instance of for durations Oct 13, 2022
@saig0 saig0 self-requested a review October 14, 2022 04:15
@saig0
Copy link
Member

saig0 commented Oct 14, 2022

@sccalabr thank you for your contribution. 🎉 I'll have a look at your PR in the next few days. 👀

@saig0 saig0 changed the title Issue 115 Instance of for durations fix: Instance of with correct duration names Oct 14, 2022
@sccalabr
Copy link
Contributor Author

Does this look good?

Copy link
Member

@saig0 saig0 left a comment

Choose a reason for hiding this comment

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

@sccalabr your changes look good. 🎉

I have a minor comment about the documentation. I'll adjust the docs myself and merge the PR. 🚀


This pull request counts toward the Hacktoberfest challenge. If you contributed four pull requests then you can complete your Camunda Hacktoberfest challenge here.

Comment on lines +234 to +238
"duration("P3M") instance of years and months duration"
// true

"duration("PT4H") instance of days and time duration"
// true
Copy link
Member

Choose a reason for hiding this comment

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

❌ The expression is wrapped into double quotes.

Suggested change
"duration("P3M") instance of years and months duration"
// true
"duration("PT4H") instance of days and time duration"
// true
duration("P3M") instance of years and months duration
// true
duration("PT4H") instance of days and time duration
// true

Copy link
Member

Choose a reason for hiding this comment

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

❌ We should update the type names in the description above.

We can replace the old names for the duration types. A user should use the official type names.

@saig0 saig0 merged commit 56874ce into camunda:main Oct 19, 2022
@saig0 saig0 mentioned this pull request Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

instance of doesn't work for durations
2 participants