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

feat: more filter options for incidents #43

Conversation

nitram509
Copy link
Contributor

refers to issue #26

I figured out, that .withProcessInstanceKey() does clash when using the same method name with more than one generic type (e.g. ProcessInstanceRecordValue and IncidenRecordValue end up in the same method signature).
Seems to be a limit of Kotlin. Also, thinking of Java users, this would lead to problems in using the fluent interface.

So, I do propose to prefix by type. Examples:

  • incidentWithProcessInstanceKey()
  • incidentWithProcessDefinitionKey()
  • incidentWithJobKey()

That should make the use from/within Java more simple and readable.

Any feedback welcome

@nitram509
Copy link
Contributor Author

Hi @berkgok @Hard-Coder05 @JubayerJoy,
you also picked one issue of this kind and I assume you faced a similar problem with the Kotlin compiler.
What are your thoughts?

@saig0 saig0 removed the request for review from ChrisKujawa October 18, 2021 09:29
@saig0 saig0 added the hacktoberfest-accepted An accepted PR during Hacktoberfest 2021 label Oct 18, 2021
Copy link
Collaborator

@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.

@nitram509 thank you for your contribution 🎉

Your RP looks good and solves the issue 👍
I have a suggestion to improve the API. Please have a look at my comment.

@nitram509 nitram509 requested a review from saig0 October 18, 2021 18:51
Copy link
Collaborator

@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.

@nitram509 thank you for your fast replay. Everything looks good 👍
I'll merge your PR 🚀

@saig0 saig0 merged commit bbf9966 into camunda-community-hub:main Oct 19, 2021
@nitram509 nitram509 deleted the issue-26-more-filter-options-for-incidents branch October 19, 2021 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest hacktoberfest-accepted An accepted PR during Hacktoberfest 2021
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants