This repository has been archived by the owner on Jul 12, 2020. It is now read-only.
New patch generation system #51
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -16,7 +16,7 @@ | |||
|
|
|||
| use QafooLabs\Refactoring\Domain\Model\EditorBuffer; | |||
| use QafooLabs\Refactoring\Domain\Model\LineRange; | |||
| use QafooLabs\Patches\PatchBuilder; | |||
| use QafooLabs\Refactoring\Adapters\PatchBuilder\PatchBuilder; | |||
There was a problem hiding this comment.
Where is the factory code for this changed?
|
Can you remove the |
|
Do you want me to remove the whole |
|
Also did you see me comment about the behat tests and the fact that they test the patch output rather than the result of the patch. Do think this would be a worthwhile change to make to the testsuite? |
|
Ok, I've removed all obsoleted code and tests. |
|
@tomphp Can you squash the commits to one? That be the only thing left :) |
|
@beberlei squashed :) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR provides a new mechanism for generating the patches. Please read below
Benefits
Implementation
The patch is generated by phpspec/php-diff which is the library phpspec uses to produce diffs
https://github.com/phpspec/php-diff
This is done via a patch building library which I have created for this purpose and put into a seperate repository as I it could be useful in other project also.
https://github.com/tomphp/PatchBuilder
Testing
One thing I which I should say is that I have had to modify the behat features to get it to pass, the reason for this is that behat is testing the actual patch output which had been changed (and optimised) in this version.
I think a better solution would be to test the results of the patches, i.e. generated the patches and hand them to the patch command then check the result. I'm not sure however if this would be possible using the virtual file system and therefore make require using the real file system.
While I'm 90% sure everything that I have done is correct, it might first be work updating the test suite with the new method i just mentioned against the existing code then running this pr against that test suite. Thoughts?
Tidying Up
This PR currently has all the left over classes from the old patch system lying around, I will tidy this up before expecting a merge.
What next?
I will compile and personally use this modded version for a while and see that it's working properly. If it is I think it would be great if we could go ahead with this version as it will allow progress with feature development.