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

Do not log by default #1472

Merged
merged 3 commits into from Jan 22, 2021
Merged

Do not log by default #1472

merged 3 commits into from Jan 22, 2021

Conversation

sanmai
Copy link
Member

@sanmai sanmai commented Jan 18, 2021

In the light of #1430 it doesn't make sense to keep Infection suggesting logging at all times. Keeping old behavior will mean that Infection will use 66% more memory where it could not.

This PR:

  • Updates configuration master to do not log by default.
  • Covered by tests

@sanmai sanmai changed the base branch from master to 0.21 January 18, 2021 14:09
@sanmai sanmai enabled auto-merge (squash) January 18, 2021 14:09
@maks-rafalko maks-rafalko added this to the 0.21.0 milestone Jan 21, 2021
@sanmai sanmai disabled auto-merge January 22, 2021 09:21
@sanmai sanmai merged commit edfe1bd into infection:0.21 Jan 22, 2021
@sanmai sanmai deleted the pr/2021-01/do-not-log branch January 22, 2021 09:21
@maks-rafalko
Copy link
Member

merged to master edfe1bd

@theofidry
Copy link
Member

I wonder if it's wise though: when you kill everything that's all good but as soon as you got one, then what? You need to run infection again with a log file this time

@sanmai
Copy link
Member Author

sanmai commented Jan 29, 2021

Then just -s, no?

@theofidry
Copy link
Member

WDYM?

@sanmai
Copy link
Member Author

sanmai commented Jan 30, 2021

I always use Infection with --show-mutations, no logging needed.

@theofidry
Copy link
Member

theofidry commented Jan 30, 2021

Ah I see. Still it's not the default this may just lead to very confused users. What do you think of enabling --show-mutations automatically there is no log file?

@sanmai
Copy link
Member Author

sanmai commented Jan 31, 2021

This also be confusing, e.g. when a user expects --show-mutations to be enabled automatically, adds a log file, and sees it no longer enabled without clear reason. This can be alleviated by a adding a note in the output, but then we could consider other similar ways. For example:

  • We could add "Tip: to show escaped mutations run Infection with --show-mutations" somewhere after the results.
  • We could have --show-mutations enabled at all times unconditionally, but showing it above metrics.

@maks-rafalko
Copy link
Member

I would vote for

We could add "Tip: to show escaped mutations run Infection with --show-mutations" somewhere after the results.

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.

None yet

3 participants