Skip to content

Run 2 enums from DataTypes#5844

Merged
jgrosseo merged 3 commits intoAliceO2Group:devfrom
ddobrigk:dev
Apr 8, 2021
Merged

Run 2 enums from DataTypes#5844
jgrosseo merged 3 commits intoAliceO2Group:devfrom
ddobrigk:dev

Conversation

@ddobrigk
Copy link
Copy Markdown
Contributor

@ddobrigk ddobrigk commented Apr 1, 2021 •
edited
Loading

@marcovanleeuwen, @mconcas, @qgp: @jgrosseo pointed out that the Run2 utils header for the ALICE3 AO2D conversion tool actually re-declared some enums that already exist in DataTypes.h, so to avoid confusion, I just re-used those and removed the double declarations.

mconcas
mconcas previously approved these changes Apr 1, 2021
View reviewed changes
@ddobrigk ddobrigk marked this pull request as draft April 1, 2021 11:20
@ddobrigk
Copy link
Copy Markdown
Contributor Author

ddobrigk commented Apr 1, 2021

Marking as draft pending the merging of a rename PR from earlier today from @jgrosseo - will require minor changes after that one is merged

@ddobrigk ddobrigk marked this pull request as ready for review April 1, 2021 17:15
@ddobrigk
Copy link
Copy Markdown
Contributor Author

ddobrigk commented Apr 1, 2021

Now compatible with this PR

@ddobrigk
Copy link
Copy Markdown
Contributor Author

ddobrigk commented Apr 6, 2021 •
edited
Loading

Hi everyone, this is just a friendly ping since this was popping up just before the easter break :-D I think it still needs approval from either @qgp or @marcovanleeuwen (not sure why @mconcas' approval is no longer good for the system... but ok)

@mconcas
Copy link
Copy Markdown
Collaborator

mconcas commented Apr 6, 2021 •
edited
Loading

Hi everyone, this is just a friendly ping since this was popping up just before the Christmas break :-D I think it still needs approval from either @qgp or @marcovanleeuwen (not sure why @mconcas' approval is no longer good for the system... but ok)

Hi David, could you please fix the errors raised by the CI? Thanks in advance.
PS: when you push other commits after an approval it automatically dismisses it as the code actually changed.

@ddobrigk
Copy link
Copy Markdown
Contributor Author

ddobrigk commented Apr 6, 2021

Hi everyone, this is just a friendly ping since this was popping up just before the Christmas break :-D I think it still needs approval from either @qgp or @marcovanleeuwen (not sure why @mconcas' approval is no longer good for the system... but ok)

Hi David, could you please fix the errors raised by the CI? Thanks in advance.
PS: when you push other commits after an approval it automatically dismisses it as the code actually changed.

Oooops, sorry, my bad - forgot an include! Now fixed.

About your P.S.: Sure, that makes sense. My comment was more regarding the fact that git says you cannot approve anymore (but previously you could) as it only mentions @qgp and @marcovanleeuwen as approvers now. No issue in any case for me of course :-D

@mconcas mconcas self-requested a review April 6, 2021 09:04
@ddobrigk
Copy link
Copy Markdown
Contributor Author

ddobrigk commented Apr 8, 2021

Hi everyone, is there something still missing before we can merge this PR?... Sorry if I miss something obvious.

@jgrosseo jgrosseo merged commit 798f7d0 into AliceO2Group:dev Apr 8, 2021
cortesep pushed a commit to cortesep/AliceO2 that referenced this pull request Jun 11, 2021
@ddobrigk @cortesep
Run 2 enums from DataTypes (AliceO2Group#5844)
9a960a0 
* Run 2 enums from DataTypes

* Update ALICE3toAO2D.C

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

Reviewers

@mconcas mconcas mconcas approved these changes

@marcovanleeuwen marcovanleeuwen Awaiting requested review from marcovanleeuwen

@qgp qgp Awaiting requested review from qgp

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ddobrigk @mconcas @jgrosseo