Skip to content

Generic framework for CF Flow in O2#6661

Closed
EmilGorm wants to merge 2 commits into
AliceO2Group:devfrom
EmilGorm:dev
Closed

Generic framework for CF Flow in O2#6661
EmilGorm wants to merge 2 commits into
AliceO2Group:devfrom
EmilGorm:dev

Conversation

@EmilGorm
Copy link
Copy Markdown
Collaborator

@EmilGorm EmilGorm commented Jul 17, 2021

No description provided.

jgrosseo
jgrosseo requested changes Jul 19, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@jgrosseo jgrosseo left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Please find some remarks here.

Later you need to run the clang formatter on all your code. Please do not do that yet, because then the already posted comments lose their connection to the code. Let's first work off the comments.

Comment thread Analysis/Core/CMakeLists.txt Outdated
Comment thread Analysis/Core/CMakeLists.txt Outdated
Comment thread Analysis/Core/include/AnalysisCore/GFW.h Outdated
Comment thread Analysis/Tasks/PWGCF/CMakeLists.txt Outdated
Comment thread Analysis/Tasks/PWGCF/GenericFramework.cxx Outdated
Comment thread Analysis/Tasks/PWGCF/GenericFramework.cxx Outdated
Comment thread Analysis/Tasks/PWGCF/GenericFramework.cxx Outdated
Comment thread Analysis/Tasks/PWGCF/GenericFramework.cxx Outdated
Comment thread Analysis/Tasks/PWGCF/GenericFramework.cxx Outdated
Comment thread Analysis/Tasks/PWGCF/GenericFramework.cxx Outdated
@EmilGorm EmilGorm closed this Jul 27, 2021
@EmilGorm EmilGorm reopened this Jul 27, 2021
@EmilGorm
Copy link
Copy Markdown
Collaborator Author

EmilGorm commented Jul 29, 2021

Hey, I was wondering if there is anything I am missing for the pull request to be resolved? I have implemented the suggestions and clang-formatted the code.

Cheers,
Emil

@jgrosseo
Copy link
Copy Markdown
Collaborator

jgrosseo commented Jul 29, 2021

It was not compiling earlier. You should check the logs.
I think by closing and reopening it, you have deleted the output of the build. I will trigger it again.

@jgrosseo
Copy link
Copy Markdown
Collaborator

jgrosseo commented Jul 29, 2021

Hi.
This error occurs when you have included AnalysisDataModel.h in any of the header files which go to the library.

/System/Volumes/Data/build/ci_checks/alice_ci_0/o2/sw/SOURCES/O2/6661/0/Framework/Core/include/Framework/Expressions.h:433:26: error: no member named 'Projector' in namespace 'gandiva' std::shared_ptr<gandiva::Projector> createProjector(gandiva::SchemaPtr const& Schema,

@jgrosseo
Copy link
Copy Markdown
Collaborator

jgrosseo commented Jul 29, 2021

You do not have that error locally?

iarsene
iarsene reviewed Aug 10, 2021
View reviewed changes
Comment thread Analysis/Tasks/PWGCF/CMakeLists.txt
# or submit itself to any jurisdiction.

add_subdirectory(FemtoDream)
add_subdirectory(GenericFramework)
Copy link
Copy Markdown
Collaborator

@iarsene iarsene Aug 10, 2021

Choose a reason for hiding this comment

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

Shouldn't this directory be named to something more specific, like "FlowGenericFramework" ?

@iarsene
Copy link
Copy Markdown
Collaborator

iarsene commented Aug 13, 2021

Hi @EmilGorm,
Are there any news on this PR?

@EmilGorm
Copy link
Copy Markdown
Collaborator Author

EmilGorm commented Aug 15, 2021

I am on vacation, I should have closed the PR, before I left. My apologies. I will reopen it, when I have fixed these issues.

@EmilGorm EmilGorm closed this Aug 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iarsene iarsene iarsene left review comments

@jgrosseo jgrosseo jgrosseo approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@EmilGorm @jgrosseo @iarsene