Skip to content
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.

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

update: gdb logging enable/disable command #1095

Merged
merged 4 commits into from May 19, 2024
Merged

update: gdb logging enable/disable command #1095

merged 4 commits into from May 19, 2024

Conversation

mtwoz
Copy link
Contributor

@mtwoz mtwoz commented Apr 26, 2024

'set logging on/off' is deprecated and replaced with 'set logging enabled on/off'.

Description

  • My code follows the code style of this project.
  • My change includes a change to the documentation, if required.
  • If my change adds new code, adequate tests have been added.
  • I have read and agree to the CONTRIBUTING document.

'set logging on/off' is deprecated and replaced with 'set logging enabled on/off'
Copy link

🤖 Coverage update for 1930e8d 🟢

Old New
Commit 92f45ba 1930e8d
Score 71.5835% 71.5835% (0)

@hugsy
Copy link
Owner

hugsy commented Apr 26, 2024

Do you happen to know in which version this deprecation occured?

Update: it seems to have appeared somewhere between v9 (doesn't work) and v12 (works).
So just to ensure retro-compatibility, please update your PR by checking the version against GDB_VERSION

@mtwoz
Copy link
Contributor Author

mtwoz commented Apr 27, 2024

Do you happen to know in which version this deprecation occured?

Update: it seems to have appeared somewhere between v9 (doesn't work) and v12 (works). So just to ensure retro-compatibility, please update your PR by checking the version against GDB_VERSION

Thanks for reminding me. So do I need to add an if judgment to choose to use 'set logging on/off' or 'set logging enabled on/off' based on GDB_VERSION?

@hugsy
Copy link
Owner

hugsy commented Apr 27, 2024

Thanks for reminding me. So do I need to add an if judgment to choose to use 'set logging on/off' or 'set logging enabled on/off' based on GDB_VERSION?

From the docs it seems that set logging enabled on/off is the way going forward so yes, it is better to simply add a if GDB_VERSION > check until our base version supported becomes one where set logging enabled on/off always works

Copy link

🤖 Coverage update for b5ffba9 🟢

Old New
Commit 18c1f7c b5ffba9
Score 71.5629% 71.5629% (0)

Copy link

🤖 Coverage update for 9a14dcf 🟢

Old New
Commit 18c1f7c 9a14dcf
Score 71.5629% 71.5629% (0)

@mtwoz
Copy link
Contributor Author

mtwoz commented May 15, 2024

Thanks for reminding me. So do I need to add an if judgment to choose to use 'set logging on/off' or 'set logging enabled on/off' based on GDB_VERSION?

From the docs it seems that set logging enabled on/off is the way going forward so yes, it is better to simply add a if GDB_VERSION > check until our base version supported becomes one where set logging enabled on/off always works

Hi, I just added those judgments mentioned above.

@hugsy
Copy link
Owner

hugsy commented May 16, 2024

@mtwoz Code formatting fails. Please fix and re-submit. Once it passes it can be merged.

@mtwoz
Copy link
Contributor Author

mtwoz commented May 17, 2024

@mtwoz Code formatting fails. Please fix and re-submit. Once it passes it can be merged.

OK. Thanks. I just fixed it.

Copy link

🤖 Coverage update for d5e460d 🟢

Old New
Commit 18c1f7c d5e460d
Score 71.5629% 71.5629% (0)

Copy link
Owner

@hugsy hugsy left a comment

Choose a reason for hiding this comment

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

LGTM

@hugsy hugsy merged commit 220611a into hugsy:main May 19, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants