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.

Sign up for GitHub

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

Jump to bottom

Use new config data when Infection has been executed in a first time (without .json or json.dist) #577

Closed
wants to merge 4 commits into from
Closed

Use new config data when Infection has been executed in a first time (without .json or json.dist) #577

wants to merge 4 commits into from

Conversation

sidz
Copy link
Member

@sidz sidz commented Nov 20, 2018

This PR fixes #576

tests-checker[bot]
tests-checker bot suggested changes Nov 20, 2018
View reviewed changes
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?

maks-rafalko
maks-rafalko reviewed Nov 20, 2018
View reviewed changes
Copy link
Member

@maks-rafalko maks-rafalko left a comment

Choose a reason for hiding this comment

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

Could you please add them to make sure everything works as expected?

I agree with that guy, can we have a test to prevent regression?

Also, could you please target 0.11 branch?

src/Config/InfectionConfig.php Outdated Show resolved Hide resolved
@sidz
Copy link
Member Author

sidz commented Nov 20, 2018

I agree with that guy, can we have a test to prevent regression?

@borNfreee Fine with me if we are agree to continue with this approach.

@sidz sidz changed the title Use the new config data when Infection has been executed in a first time (without .json or json.dist) Use new config data when Infection has been executed in a first time (without .json or json.dist) Nov 20, 2018
@sidz sidz force-pushed the use-new-config-data-after-first-execution branch from 384c2a9 to fec5ee8 Compare November 20, 2018 20:22
@sidz sidz changed the base branch from master to 0.11 November 20, 2018 20:22
@maks-rafalko
Copy link
Member

@borNfreee Fine with me if we are agree to continue with this approach.

👍 from me

@sidz sidz force-pushed the use-new-config-data-after-first-execution branch 4 times, most recently from b9fdbdd to 41cd291 Compare November 25, 2018 19:38
@sidz sidz removed the In Progress label Nov 25, 2018
@maks-rafalko
Copy link
Member

sidz removed the in progress label a day ago

something is wrong in Travis when phpdbg is used. I have restarted the builds but it doesn't help.

@sidz
Copy link
Member Author

sidz commented Nov 26, 2018

@borNfreee yeah I know. It smth what I broke in this PR but didn't find what exactly yet

@maks-rafalko
Copy link
Member

ok, I thought it's ready because of removed "in progress" label ( ͡° ͜ʖ ͡°)

@sanmai sanmai mentioned this pull request Nov 27, 2018
Closed
@sidz
Copy link
Member Author

sidz commented Dec 6, 2018

had another one round of checking what is going on. Without success. Any help?

@maks-rafalko
Copy link
Member

maks-rafalko commented Feb 16, 2019 •
edited

had another one round of checking what is going on. Without success. Any help?

@sidz could you please rebase and target 0.12 branch? I will see how I can help to debug travis failures

@sanmai sanmai mentioned this pull request Mar 4, 2019
Merged
@sidz sidz force-pushed the use-new-config-data-after-first-execution branch from e1a0499 to 39f9b2f Compare March 4, 2019 06:14
@sidz sidz changed the base branch from 0.11 to 0.12 March 4, 2019 06:14
@maks-rafalko
Copy link
Member

Superseded by #651.

Thank you @sidz

@ghost ghost removed the In Progress label Mar 29, 2019
@maks-rafalko maks-rafalko deleted the use-new-config-data-after-first-execution branch March 29, 2019 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maks-rafalko maks-rafalko maks-rafalko left review comments

@tests-checker tests-checker[bot] tests-checker requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sidz @maks-rafalko