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

Fix the order in which the infection configuration files are loaded #1105

Merged
merged 1 commit into from Mar 3, 2020

Conversation

theofidry
Copy link
Member

The expected order is:

  • custom file if specified
  • infection.json file if available
  • infection.json.dist file

Copy link

@tests-checker tests-checker bot left a comment

Choose a reason for hiding this comment

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

Thank you for contributing to Infection. We noticed you didn't add any tests. Could you please add them to make sure everything works as expected?

@theofidry theofidry merged commit 8043b83 into infection:master Mar 3, 2020
@theofidry theofidry deleted the bugfix/config-order branch March 3, 2020 09:10
@maks-rafalko maks-rafalko added this to the 0.16.0 milestone Mar 3, 2020
@maks-rafalko
Copy link
Member

I guess this is BC break. Nothing bad here as we are on 0.x, but let's mark such issues so that we can list it on the blog post for 0.16 version / changelog

@theofidry
Copy link
Member Author

Fair

@sanmai
Copy link
Member

sanmai commented Mar 3, 2020

This isn’t a BC break, it’s a bug fix. Hard to BC break something that wasn’t working.

@maks-rafalko
Copy link
Member

Why so? If you had 2 files (with and without .dist) and they had different content, they will be swapped right now, no?

@sanmai
Copy link
Member

sanmai commented Mar 4, 2020

If you had infection.json before together with infection.json.dist the first was never ever considered. Contrary to what supposed to happen according to the manual.

You can commit it to the VCS and, if necessary, override it locally by creating infection.json which should be ignored (e.g. in .gitignore).

I wish this was tested somehow.

@maks-rafalko
Copy link
Member

If you had infection.json before together with infection.json.dist the first was never ever considered.

right, and now (after this PR) it will be considered.

@sanmai
Copy link
Member

sanmai commented Mar 4, 2020

Right, means I'm wrong that this isn't a BC break. It is both bug fix and BC break.

@maks-rafalko
Copy link
Member

Exactly. Thanks ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants