Skip to content

G-UTILS-GPROFILER REWORK (5): Rework complex files#928

Closed
granulatedekel wants to merge 2 commits intorefactor-into-g-utils-condensedfrom
refactor-into-g-utils-condensed-complex
Closed

G-UTILS-GPROFILER REWORK (5): Rework complex files#928
granulatedekel wants to merge 2 commits intorefactor-into-g-utils-condensedfrom
refactor-into-g-utils-condensed-complex

Conversation

@granulatedekel
Copy link
Copy Markdown

@granulatedekel granulatedekel commented Sep 23, 2024

Part of the G-UTILS-GPROFILER REWORK saga - which aims to move away shared code from gprofiler to the g-utils project:
intel/granulate-utils#263 intel/granulate-utils#261 #925 #926 #926

This is the ultimate PR which is responsible for handling the last few files that required logical changes as well

@granulatedekel granulatedekel changed the title rework complex files to use granulate-utils G-UTILS REWORK (3): Rework complex files Sep 23, 2024
@granulatedekel granulatedekel changed the title G-UTILS REWORK (3): Rework complex files G-UTILS-GPROFILER REWORK (5): Rework complex files Sep 23, 2024
@granulatedekel granulatedekel added the g-utils-gprofiler-rework PRs related to the G-UTILS-GPROFILER saga label Sep 23, 2024
@granulatedekel granulatedekel marked this pull request as ready for review September 23, 2024 22:52
@roi-granulate
Copy link
Copy Markdown
Contributor

roi-granulate commented Oct 1, 2024

@granulatedekel I’m getting into this here
but before I dig into the code - shouldn’t this PR CI be ultimately green?
because PR no.5 should point to granulate-utils’ PR no. 2, which, as I understand, should represent the full code merged into both repo’s main branches (== the final product)
what am I missing?

@YoniKF
Copy link
Copy Markdown
Contributor

YoniKF commented Oct 1, 2024

@roi-granulate

@granulatedekel I’m getting into this here but before I dig into the code - shouldn’t this PR CI be ultimately green? because PR no.5 should point to granulate-utils’ PR no. 2, which, as I understand, should represent the full code merged into both repo’s main branches (== the final product) what am I missing?

Linter failure:

ERROR: /home/runner/work/gprofiler/gprofiler/gprofiler/utils/__init__.py Imports are incorrectly sorted and/or formatted.
Skipped 2 files
ERROR: /home/runner/work/gprofiler/gprofiler/gprofiler/profilers/java.py Imports are incorrectly sorted and/or formatted.

Probably due to bulk rename of modules without reordering. @granulatedekel will fix but I don't think that this blocks review.

@YoniKF YoniKF requested a review from roi-granulate October 1, 2024 10:16
@roi-granulate
Copy link
Copy Markdown
Contributor

@YoniKF indeed minor fix - but it blocks tests from running, so we have no indication on breakages.
I will start reviewing, but please fix this.

from gprofiler.consts import CPU_PROFILING_MODE
from gprofiler.platform import is_linux, is_windows
from granulate_utils.gprofiler.platform import is_windows
import granulate_utils.gprofiler.utils as _utils
Copy link
Copy Markdown
Contributor

@Jongy Jongy Oct 8, 2024

Choose a reason for hiding this comment

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

Note that moving run_process to granulate-utils poses a problem, since in another PR we're adding more gprofiler-specific behavior to that function.

@YoniKF
Copy link
Copy Markdown
Contributor

YoniKF commented Apr 15, 2026

Can someone with write access to the repo close this PR?
It appears in my https://github.com/pulls/involves page and I cannot remove it otherwise.
Maybe @mlim19?

@mlim19 mlim19 closed this Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

g-utils-gprofiler-rework PRs related to the G-UTILS-GPROFILER saga

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants