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

Freight analysis #26

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Freight analysis #26

wants to merge 3 commits into from

Conversation

danielundgit
Copy link

duplicated ExtractPersonEvents (called ExtractFreightEvents) to get freight or in general event type activities out from an event file

Copy link
Contributor

@simei94 simei94 left a comment

Choose a reason for hiding this comment

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

pls adapt your code such that it checks all the sonar cloud requirements

@GregorRyb
Copy link
Contributor

@simei94 I think not having a test for this analysis is ok...

@GregorRyb GregorRyb closed this May 25, 2023
@GregorRyb GregorRyb reopened this May 25, 2023
@GregorRyb
Copy link
Contributor

GregorRyb commented May 31, 2023

Actually there is a bug you should fix before we can merge it. Also I would suggest that you just use the ActivityStartEventHandler for example, instead of all events and then filter for freight. Then you could clean up your code a bit more. If you need help let us know. Thanks!

@kainagel
Copy link
Contributor

@rakow : Könntest Du da bitte mal reinschauen? Meine Intuition ist, dass wir event.getAttributes() von außen nicht verwenden sollten; stattessen instanceof ..., dann casten, dann typisierter Zugriff. Man könnte versuchen, getAttributes() protected zu machen; wird nur an ganz wenigen Stellen verwendet, und die könnte man in die gleiche package moven. (Leider gibt es core.events und api.core...events ... my (very old) fault.)

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.

4 participants