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

MDBF-994: MTR Generator to unify mtr invocation in Buildbot #722

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

Conversation

cvicentiu
Copy link
Member

@cvicentiu cvicentiu commented Mar 4, 2025

This PR introduces an MTR Generator so we can standardise MTR invocation. It's likely not in its final form, we will likely have to add ENVIRONMENT variables support, for MTR_FEEDBACK_PLUGIN=1 for instance, but this will happen when we have specific builders use this class. For now the code is not used in any buildbot config, although it does have unit tests.

Refactoring:
Extract the base common logic from CMakeGenerator and put it into the "base" folder instead.

@cvicentiu cvicentiu force-pushed the mtr_generator branch 2 times, most recently from d67e0e8 to a3e7357 Compare March 4, 2025 06:13
This is in preparation for sharing code for MTRGenerator
@cvicentiu cvicentiu force-pushed the mtr_generator branch 2 times, most recently from de1962a to cec6f98 Compare March 4, 2025 06:23
This class creates typesafety much like CMake Generator and allows us
to ensure we run all MTR commands in the same format, with parameters
sorted. Typos also cause immediate script validation failure, rather
than a failure at runtime.
@cvicentiu cvicentiu marked this pull request as ready for review March 4, 2025 06:29
@cvicentiu cvicentiu changed the title Mtr generator MDBF-994: MTR Generator to unify mtr invocation in Buildbot Mar 4, 2025
@cvicentiu
Copy link
Member Author

@RazvanLiviuVarzaru The pre-commit failures are actually false positives. Unit tests pass so imports are ok. The assertIn suggestion is incorrect as it's suggesting that I change a function name.

@fauust
Copy link
Collaborator

fauust commented Mar 6, 2025

@RazvanLiviuVarzaru The pre-commit failures are actually false positives. Unit tests pass so imports are ok. The assertIn suggestion is incorrect as it's suggesting that I change a function name.

Easy to fix (please do), see https://github.com/MariaDB/buildbot/blob/dev/.codespellrc.

Also, if E0402 is wrong, we should disable it, either everywhere or only in that file.

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.

2 participants