Skip to content

Conversation

@angirar
Copy link

@angirar angirar commented Nov 12, 2024

No description provided.

@spagangriso
Copy link

Tagging @madbaron , @kkrizka ; this PR implements a couple of things, but mainly we have adapted the timing hit filter so that it works also in cases where 1 sim hit <-> 1 reco hit assumption is not true anymore.

I'm not 100% sure on the changes on the CMakeLists.txt file; I wonder if we have some good example we should follow for the current setup of packages.

@madbaron
Copy link

I don't think we have an example set up yet, but perhaps I can invoke @tmadlener and ask him to take a look specifically at CMakeLists.txt?

Copy link

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

You will probably have to rebase this onto the master branch after #20 (or merge that in). From a quick look, there is a chance for merge conflicts in the process. Let me know if you need help with that.

I have otherwise effectively only looked at the changes for the CMakeLists.txt

Comment on lines -23 to -38
# load default settings from ILCSOFT_CMAKE_MODULES
INCLUDE( ilcsoft_default_settings )

SET( COMPILER_FLAGS "-Wno-effc++" )
MESSAGE( STATUS "FLAGS ${COMPILER_FLAGS}" )
FOREACH( FLAG ${COMPILER_FLAGS} )
CHECK_CXX_COMPILER_FLAG( "${FLAG}" CXX_FLAG_WORKS_${FLAG} )
IF( ${CXX_FLAG_WORKS_${FLAG}} )
MESSAGE ( STATUS "Adding ${FLAG} to CXX_FLAGS" )
### We append the flag, so they overwrite things from somewhere else
SET ( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${FLAG} ")
ELSE()
MESSAGE ( STATUS "NOT Adding ${FLAG} to CXX_FLAGS" )
ENDIF()
ENDFOREACH()

Choose a reason for hiding this comment

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

I would keep these. With #20 the warnings should be fixed so that output should be manageable again.

Comment on lines -131 to +114
# needed for adding header files to xcode project
IF(CMAKE_GENERATOR MATCHES "Xcode")
FILE( GLOB_RECURSE library_headers "*.h" )
ADD_SHARED_LIBRARY( ${PROJECT_NAME} ${library_sources} ${library_headers})
ELSE()
ADD_SHARED_LIBRARY( ${PROJECT_NAME} ${library_sources} )
ENDIF()

INSTALL_SHARED_LIBRARY( ${PROJECT_NAME} DESTINATION lib )

# display some variables and write them to cache
DISPLAY_STD_VARIABLES()
ADD_LIBRARY( ${PROJECT_NAME} SHARED ${library_sources} )

Choose a reason for hiding this comment

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

No strong opinion on removing this. It's still present in iLCSoft/MarlinTrkProcessors. However what definitely has to stay is the installation of the library that we build, as otherwise we will not be able to use the processors when we use this into a software stack.

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