Use native Ruby implementations in Yast::URL and Yast::IP #310
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
| URLRecode.EscapeQuery(Ops.get_string(tokens, "query", "")) | ||
| ) | ||
| end | ||
| uri = URI() |
There was a problem hiding this comment.
This does not work. It invokes the method Kernel.URI which needs one argument.
There was a problem hiding this comment.
why tests doesn't failed? any bug that do not cause fail of tests show problem in test coverage.
There was a problem hiding this comment.
The tests do fail: https://travis-ci.org/yast/yast-yast2/builds/44472406
|
Trying to let all tests in the old testsuite pass is surprisingly quite complicated. There are more of thos tiny differences in behaviour than expected. |
Actually this was done with
git merge -X ours --no-commit master
and then
for F in library/types/src/modules/URL.rb library/types/test/url_test.rb; do
git show HEAD:$F > $F
git add $F
done
to recover from the revert of these two files in master.
This was trickier than I thought :-/
|
In this PR we have found that replacing the URL implementation is hard. If we don't have bugs/features to address then let's not continue in the refactoring. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 is a continuation of #305, using a branch in the shared repo so that more people can contribute commits to the PR.