Skip to content

Conversation

@vkucera
Copy link
Collaborator

@vkucera vkucera commented Dec 8, 2025

Requires #14211

@github-actions github-actions bot added the pwglf label Dec 8, 2025
@github-actions github-actions bot changed the title Delete unused source files [PWGLF] Delete unused source files Dec 8, 2025
@vkucera vkucera marked this pull request as ready for review December 8, 2025 22:16
Copy link
Collaborator

@romainschotter romainschotter left a comment

Choose a reason for hiding this comment

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

Hi @vkucera !
For what concerns the strangeness, I am not entirely sure why we would like to remove these converters. Usually, converters are meant to ensure backward compatibility, particularly for cases where derived data were produced with an old tag (for debugging or maybe because there is no benefit in regenerating them) but are then analyzed with a newer tag. Could you please elaborate on the motivation for removing them?

@vkucera
Copy link
Collaborator Author

vkucera commented Dec 8, 2025

Hi @vkucera ! For what concerns the strangeness, I am not entirely sure why we would like to remove these converters. Usually, converters are meant to ensure backward compatibility, particularly for cases where derived data were produced with an old tag (for debugging or maybe because there is no benefit in regenerating them) but are then analyzed with a newer tag. Could you please elaborate on the motivation for removing them?

Hi @romainschotter , the motivation is in the PR title. These files are unused.

@romainschotter
Copy link
Collaborator

I am still not sure to understand why their removal would be beneficial

@vkucera vkucera marked this pull request as draft December 12, 2025 13:51
@vkucera
Copy link
Collaborator Author

vkucera commented Dec 12, 2025

The deleted converters cannot be used in the current setup because there are no CMake commands to compile them. I propose the following:

  • If they should be usable by default, include them in compilation.
  • If they should be usable on demand, add the CMake commands but comment them out.
  • If they are not needed, delete them.

Please let me know what you prefer. In case you wish to keep them, please make sure that the bugs currently reported by the analysis tools are fixed soon.

@romainschotter
Copy link
Collaborator

Hi @vkucera !
As discussed during the ALICE week, it is by mistake that these converters have no corresponding CMake commands. We should thus include them in the CMakeLists.txt.
Do you want to me take care of it? Or do you wish to do it?

@vkucera
Copy link
Collaborator Author

vkucera commented Dec 15, 2025

Hi @romainschotter , I tried to add the CMake commands but stradautracksconverter.cxx doesn't compile because the table filling doesn't match the table columns.
Can you please fix it?

@vkucera
Copy link
Collaborator Author

vkucera commented Dec 19, 2025

Restored the deleted converters.

@vkucera vkucera marked this pull request as ready for review December 19, 2025 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants