Skip to content

Gecko reporter #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pablogsal wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Gecko reporter #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
35c4e5c
gh-135953: Add Gecko format to sampling profiler
ivonastojanovic Sep 28, 2025
4947d41
Add NEWS entry
pablogsal Oct 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Lib/profiling/sampling/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from .collector import Collector
from .pstats_collector import PstatsCollector
from .stack_collector import CollapsedStackCollector
from .gecko_collector import GeckoCollector
Copy link
Owner Author

@pablogsal pablogsal Sep 27, 2025

Choose a reason for hiding this comment

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

ℹ️ Testing

The addition of GeckoCollector to the imports suggests a new feature or functionality. Ensure that the GeckoCollector class is well-documented and tested. Consider adding unit tests to cover its functionality if not already present.

Copy link
Owner Author

@pablogsal pablogsal Sep 27, 2025

Choose a reason for hiding this comment

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

⚠️ Testing

Ensure that the GeckoCollector module is correctly implemented and imported. If this module is not present or has issues, it could lead to runtime errors when attempting to use it. Consider adding tests to verify its functionality.

Suggested fix:

Suggested change
from .gecko_collector import GeckoCollector
from .gecko_collector import GeckoCollector

Copy link
Owner Author

@pablogsal pablogsal Sep 27, 2025

Choose a reason for hiding this comment

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

ℹ️ Testing

The addition of GeckoCollector to the imports and __all__ list suggests a new feature or functionality. Ensure that GeckoCollector is properly tested and documented. Consider adding unit tests if they are not already present to verify its behavior and integration with the existing system.

Copy link
Owner Author

@pablogsal pablogsal Sep 27, 2025

Choose a reason for hiding this comment

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

⚠️ Testing

Ensure that the GeckoCollector module is properly implemented and tested. Adding new imports without corresponding tests can lead to undetected issues if the module has bugs or integration problems.

Copy link
Owner Author

@pablogsal pablogsal Sep 27, 2025

Choose a reason for hiding this comment

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

ℹ️ Maintainability

Consider adding a comment or docstring to explain the purpose of importing GeckoCollector. This can improve code readability and maintainability by providing context for future developers.

from .string_table import StringTable

__all__ = ("Collector", "PstatsCollector", "CollapsedStackCollector", "StringTable")
__all__ = ("Collector", "PstatsCollector", "CollapsedStackCollector", "GeckoCollector", "StringTable")
Copy link
Owner Author

@pablogsal pablogsal Sep 27, 2025

Choose a reason for hiding this comment

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

ℹ️ Maintainability

The __all__ list has been updated to include GeckoCollector. Ensure that this addition is intentional and that GeckoCollector is meant to be part of the public API. If GeckoCollector is not intended for public use, consider removing it from __all__.

Copy link
Owner Author

@pablogsal pablogsal Sep 27, 2025

Choose a reason for hiding this comment

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

ℹ️ Maintainability

The __all__ declaration is updated to include GeckoCollector. Ensure that this is intentional and that GeckoCollector is meant to be part of the public API. If not, consider removing it from __all__.

Suggested fix:

Suggested change
__all__ = ("Collector", "PstatsCollector", "CollapsedStackCollector", "GeckoCollector", "StringTable")
__all__ = ("Collector", "PstatsCollector", "CollapsedStackCollector", "GeckoCollector", "StringTable")

Copy link
Owner Author

@pablogsal pablogsal Sep 27, 2025

Choose a reason for hiding this comment

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

ℹ️ Maintainability

The __all__ list has been updated to include GeckoCollector. This is a good practice to control what is exported when import * is used. Ensure that GeckoCollector is intended to be part of the public API and that its interface is stable.

Copy link
Owner Author

@pablogsal pablogsal Sep 27, 2025

Choose a reason for hiding this comment

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

ℹ️ Maintainability

Including GeckoCollector in __all__ is a good practice as it explicitly defines the public API of the module. Ensure that GeckoCollector is well-documented and its interface is stable.

Copy link
Owner Author

@pablogsal pablogsal Sep 27, 2025

Choose a reason for hiding this comment

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

⚠️ Testing

Ensure that GeckoCollector is correctly implemented and tested. Adding a new component to __all__ exposes it as part of the module's public API, so it's important to verify its stability and correctness.

Loading
Loading