Skip to content

Conversation

@sjzhu0v0
Copy link
Contributor

@sjzhu0v0 sjzhu0v0 commented Feb 3, 2025

FT0C occupancy has been added to PWGDQ/DataModel. The corresponding modification has been added to tableMaker.cxx.

@github-actions github-actions bot changed the title FT0C occupancy added to PWGDQ code [PWGDQ] FT0C occupancy added to PWGDQ code Feb 3, 2025
@iarsene
Copy link
Collaborator

iarsene commented Feb 4, 2025

Hi @sjzhu0v0,
I don't really like creating a new table with just one column. This could lead to lots of tables in the future. So I suggest to add this column into the multPV table. As a temporary solution, in order to not break backward compatibility for older skims, you can make a temporary converter task for this table.

@sjzhu0v0
Copy link
Contributor Author

sjzhu0v0 commented Feb 4, 2025

Hi @sjzhu0v0, I don't really like creating a new table with just one column. This could lead to lots of tables in the future. So I suggest to add this column into the multPV table. As a temporary solution, in order to not break backward compatibility for older skims, you can make a temporary converter task for this table.

Hi, @iarsene . Thanks for your suggestion! Which folder should I add such a converter to? Please kindly give me a recommendation.

@iarsene
Copy link
Collaborator

iarsene commented Feb 4, 2025

Hi, you can add it to Tasks

@iarsene
Copy link
Collaborator

iarsene commented Feb 4, 2025

Also, @sjzhu0v0, please be aware that there is another change for this table being prepared. The PR is nearly ready, it should be merged immediately after we merge your change. So, after you merge your PR, wait a bit before yiu create new skims so that the other updates are merged

@sjzhu0v0 sjzhu0v0 closed this Feb 4, 2025
@sjzhu0v0
Copy link
Contributor Author

sjzhu0v0 commented Feb 5, 2025

Hi, @iarsene . I've created a new pull requeset. #9760 . Please take a look and close this pull requeset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants