Skip to content

DXP-2564 Implement settings command - #10

Merged
valkovich-ds merged 10 commits into
add-run-command-2from
add-settings-command
Feb 11, 2026
Merged

DXP-2564 Implement settings command#10
valkovich-ds merged 10 commits into
add-run-command-2from
add-settings-command

Conversation

@valkovich-ds

Copy link
Copy Markdown
Contributor

📋 Type of the Changes

  • Breaking change
  • Non-breaking change
  • Bug fix / minor change

🛠 Changes being made

  • ... .
  • ... .

✅ Checklist

  • My code follows the code standards of this project
  • Changed code is covered with unit tests
  • I have updated READMEs and java docs (if applicable)

@valkovich-ds
valkovich-ds requested review from a team as code owners February 3, 2026 06:44
@@ -0,0 +1,4 @@
package com.streamx.cli.commands.settings.list;

public record Property(String key, String value) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need 2 Data models for settings property? What is the difference between this record and GetCommandResult?

mixinStandardHelpOptions = true,
description = "Get configuration property"
)
public class GetCommand extends AbstractCommand<GetCommandResult> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In my opinion this command should return only value. @marekczajkowski WDYT?

mixinStandardHelpOptions = true,
description = "Display configuration properties"
)
public class ListCommand extends AbstractCommand<List<Property>> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This command should return Map. @marekczajkowski WDYT?

Comment thread streamx-cli/pom.xml Outdated
<surefire-plugin.version>3.2.5</surefire-plugin.version>
<compiler-plugin.version>3.13.0</compiler-plugin.version>
<checkstyle-plugin.version>3.6.0</checkstyle-plugin.version>
<argLine>--add-opens java.base/java.lang=ALL-UNNAMED</argLine>

@kamilchociej kamilchociej Feb 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need this? Leave in-code comment with the reason.

public class ListCommand extends AbstractCommand<List<Property>> {
@Override
public String getTextOutput(CommandResult<List<Property>> result) {
if (result.result.isEmpty()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should rename result field to data and use encapsulation via getter method

@valkovich-ds valkovich-ds changed the title Add settings command DXP-2564 Add settings command Feb 11, 2026
@valkovich-ds
valkovich-ds merged commit 8794beb into add-run-command-2 Feb 11, 2026
2 checks passed
@valkovich-ds
valkovich-ds deleted the add-settings-command branch February 11, 2026 13:53
@valkovich-ds valkovich-ds changed the title DXP-2564 Add settings command DXP-2564 Implement settings command Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants