Skip to content

fix(dotnet): remove wrong flags add from rtk#745

Closed
aeppling wants to merge 2 commits intodevelopfrom
fix/issue-572/remove-dotnet-flags
Closed

fix(dotnet): remove wrong flags add from rtk#745
aeppling wants to merge 2 commits intodevelopfrom
fix/issue-572/remove-dotnet-flags

Conversation

@aeppling
Copy link
Contributor

rtk was injecting --nologo and --logger flag, but those aren't always support in all environments.

--nologo removed -> added for compress, but is failing, a compress should not make cmd fail

--logger -> reformat the output for LLM, should not be done by rtk and is failing

Summary

  • Remove those flag addition in test methods

Test plan

  • cargo fmt --all && cargo clippy --all-targets && cargo test
  • Manual testing: rtk <command> output inspected

Important: All PRs must target the develop branch (not master).
See CONTRIBUTING.md for details.

rtk was injecting --nologo and --logger flag, but those aren't always support in all environments.

--nologo removed -> added for compress, but is failing, a compress should not make cmd fail

--logger -> reformat the output for LLM, should not be done by rtk and is failing

Signed-off-by: aesoft <43991222+aeppling@users.noreply.github.com>
@aeppling aeppling force-pushed the fix/issue-572/remove-dotnet-flags branch from e943472 to 35688e8 Compare March 20, 2026 08:48
Signed-off-by: aesoft <43991222+aeppling@users.noreply.github.com>
@aeppling
Copy link
Contributor Author

#572 closing this issue on merge

@aeppling aeppling self-assigned this Mar 20, 2026
@danielmarbach
Copy link

I don't think this is valid. The entire premise of the test parsing simplification is having a try file and at first glance this seems to be removing it (but I need to have a closer look)

@danielmarbach
Copy link

I now understand the change better. I think this introduceds a regression. Classic VSTest projects will lose the auto-injected trx logger which means RTK falls back to text parsing rather than structure TRX data. That's a degradation of the previously introduced functionality.

The right solution is conditional injection based on MTP detection rather than all-or-nothing.

I can send in a draft PR

@danielmarbach
Copy link

See #746

@aeppling
Copy link
Contributor Author

After looking at your PR i'm ok with your fix, keeping the flag and detecting whether or not we should inject those flags.

I'm closing this one, we'll move to discuss on #746

Thanks Daniel,

@aeppling aeppling closed this Mar 20, 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