Skip to content

Conversation

@vkucera
Copy link
Collaborator

@vkucera vkucera commented Jun 11, 2025

No description provided.

@github-actions
Copy link

github-actions bot commented Jun 11, 2025

O2 linter results: ❌ 36 errors, ⚠️ 0 warnings, 🔕 0 disabled

@vkucera vkucera changed the title Include What You Use [Tools] Include What You Use Jun 11, 2025
@vkucera vkucera mentioned this pull request Jun 11, 2025
@vkucera vkucera marked this pull request as ready for review June 11, 2025 12:54
Tools/ML/model.h Outdated
#define TOOLS_ML_MODEL_H_

// C++ and system includes
#include <onnxruntime_c_api.h>
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It provides OrtAllocatorType and OrtMemType.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be inside the ifdef?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the else branch?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In both cases onnxruntime_c_api is included so I removed it.

// ONNX includes
#include "Tools/ML/model.h"

#include <onnxruntime_c_api.h>
Copy link
Member

Choose a reason for hiding this comment

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

Also this one, I guess. It should be inside the ifdef.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It provides GraphOptimizationLevel, ORT_LOGGING_LEVEL_WARNING.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed

Tools/ML/model.h Outdated

// ROOT includes
#include "TSystem.h"
#include <fairlogger/Logger.h>
Copy link
Member

Choose a reason for hiding this comment

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

This one should be fixed in Framework/Logger.h

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, I'll wait for the PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@vkucera vkucera marked this pull request as draft June 11, 2025 13:27
@vkucera vkucera marked this pull request as ready for review June 13, 2025 12:43
@vkucera vkucera requested a review from ktf June 19, 2025 14:44
@ddobrigk ddobrigk enabled auto-merge (squash) June 24, 2025 09:13
@ddobrigk ddobrigk merged commit 774e2ce into AliceO2Group:master Jun 24, 2025
13 of 14 checks passed
@vkucera vkucera deleted the iwyu-Tools branch June 24, 2025 09:26
jpxrk pushed a commit to jpxrk/O2Physics that referenced this pull request Jul 16, 2025
prottayCMT pushed a commit to prottayCMT/O2Physics2024 that referenced this pull request Jul 18, 2025
JimunLee pushed a commit to JimunLee/O2Physics that referenced this pull request Jul 22, 2025
vojmach pushed a commit to vojmach/O2Physics that referenced this pull request Jul 23, 2025
jloemker pushed a commit to jloemker/O2Physics that referenced this pull request Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants