Fix code review issues: matrix parsing, Qt6 compatibility, and performance optimizations

Co-authored-by: johndoe6345789 <224850594+johndoe6345789@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2025-12-27 03:10:27 +00:00
parent a903aae859
commit 18f8bf99b2
6 changed files with 37 additions and 11 deletions

View File

@@ -23,6 +23,10 @@ public:
void cleanup() override;
private:
static constexpr int STEP_TIMEOUT_MS = 300000; // 5 minutes
static constexpr int PREPARE_TIMEOUT_MS = 60000; // 1 minute
static constexpr int CLEANUP_TIMEOUT_MS = 30000; // 30 seconds
QString m_containerId;
QString m_containerRuntime; // "docker" or "podman"

View File

@@ -46,6 +46,7 @@ private:
std::unique_ptr<core::RepoManager> m_repoManager;
std::unique_ptr<core::JobExecutor> m_executor;
std::unique_ptr<core::WorkflowParser> m_parser;
};
} // namespace gui

View File

@@ -26,11 +26,14 @@ bool ContainerBackend::executeStep(const core::WorkflowStep& step,
if (!step.run.isEmpty()) {
QProcess process;
QStringList args;
args << "exec" << m_containerId << "sh" << "-c" << step.run;
// Use specified shell or default to sh
QString shell = step.shell.isEmpty() ? "sh" : step.shell;
args << "exec" << m_containerId << shell << "-c" << step.run;
process.start(m_containerRuntime, args);
if (!process.waitForFinished(300000)) { // 5 minutes
if (!process.waitForFinished(STEP_TIMEOUT_MS)) {
emit error("Step execution timeout");
return false;
}
@@ -61,7 +64,7 @@ bool ContainerBackend::prepareEnvironment(const QString& runsOn) {
process.start(m_containerRuntime, args);
if (!process.waitForFinished(60000)) {
if (!process.waitForFinished(PREPARE_TIMEOUT_MS)) {
emit error("Container creation timeout");
return false;
}
@@ -83,7 +86,7 @@ void ContainerBackend::cleanup() {
args << "rm" << "-f" << m_containerId;
process.start(m_containerRuntime, args);
process.waitForFinished(30000);
process.waitForFinished(CLEANUP_TIMEOUT_MS);
m_containerId.clear();
}

View File

@@ -1,5 +1,6 @@
#include "core/MatrixStrategy.h"
#include "core/WorkflowParser.h"
#include <QMetaType>
#include <QDebug>
namespace gwt {
@@ -66,7 +67,7 @@ QList<QVariantMap> MatrixStrategy::generateCombinations(const QVariantMap& matri
for (const QString& key : keys) {
QVariant value = matrix[key];
if (value.type() == QVariant::List) {
if (value.typeId() == QMetaType::QVariantList) {
valueLists << value.toList();
} else {
valueLists << QVariantList{value};

View File

@@ -118,8 +118,25 @@ Workflow WorkflowParser::parse(const QString& filePath) {
if (jobNode["strategy"]) {
YAML::Node strategyNode = jobNode["strategy"];
if (strategyNode["matrix"]) {
// Store as QVariantMap for processing by MatrixStrategy
job.strategy["matrix"] = "present";
YAML::Node matrixNode = strategyNode["matrix"];
QVariantMap matrixMap;
for (auto it = matrixNode.begin(); it != matrixNode.end(); ++it) {
QString key = QString::fromStdString(it->first.as<std::string>());
YAML::Node valueNode = it->second;
if (valueNode.IsSequence()) {
QVariantList values;
for (size_t i = 0; i < valueNode.size(); ++i) {
values << QString::fromStdString(valueNode[i].as<std::string>());
}
matrixMap[key] = values;
} else {
matrixMap[key] = QString::fromStdString(valueNode.as<std::string>());
}
}
job.strategy["matrix"] = matrixMap;
}
}

View File

@@ -20,6 +20,7 @@ MainWindow::MainWindow(QWidget* parent)
: QMainWindow(parent)
, m_repoManager(std::make_unique<core::RepoManager>())
, m_executor(std::make_unique<core::JobExecutor>())
, m_parser(std::make_unique<core::WorkflowParser>())
{
setupUI();
loadRepositories();
@@ -164,12 +165,11 @@ void MainWindow::onRunWorkflow() {
m_outputView->append("\n=== Running workflow: " + workflowPath + " ===\n");
// Parse and execute
core::WorkflowParser parser;
core::Workflow workflow = parser.parse(workflowPath);
core::Workflow workflow = m_parser->parse(workflowPath);
if (parser.hasErrors()) {
if (m_parser->hasErrors()) {
m_outputView->append("Parsing errors:");
for (const QString& error : parser.getErrors()) {
for (const QString& error : m_parser->getErrors()) {
m_outputView->append(" " + error);
}
return;