Simplify code thanks to migration from v0.4 #179
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
Closed
|
There seems to be a hang in the v0.6 tests. I'll investigate that. |
|
@tonyhffong @Michael-Klassen @TeroFrondelius This is ready for review/merge. |
TotalVerb
changed the title
| line = "" | ||
| while line != "\n" | ||
| line = "." | ||
| while !isempty(line) |
There was a problem hiding this comment.
My guess is that this change hangs the lintserver().
There was a problem hiding this comment.
This change was needed to prevent a hang. On v0.6, readline changed to remove trailing newlines.
There was a problem hiding this comment.
My bad, I was late with my review, I thought the lintserver hang was still a problem.
|
For me this looks good. Although my competence is not enough to review all the changes and their possible effects. Then again code test coverage is relatively high, if tests pass then I want to believe this is not breaking anything. |
TeroFrondelius
approved these changes
Mar 6, 2017
6 tasks
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.
The first commit deletes a lot of code to guess the return types of function calls in cases where inference can handle it.
The second commit replaces
evaltypeandparsetypewith a single function that is more general and sophisticated than the existing one.The third commit deletes a large amount of 0.4-specific code that is no longer necessary, and deletes some uses of
evalwhere safer, less dynamic infrastructure exists.The fourth commit fixes bug #167 and also replaces some uses of
typeof(x) == Twithisa(x, T).The fifth commit includes tests for all the bugs fixed by this PR, except #167 which is tested in test/docs.jl.
The sixth commit improves the detection of when a type is considered mutable, which fixes several old issues; both #56 and #84 are fixed.
The seventh commit removes code specific to 0.4 dealing with vect, which is now well-defined and legal. This fixes #81 and #195.
The eighth and ninth commits fix
Dictlinting on v0.6, which was broken due to a change in theExprrepresentation.The tenth commit disables some tests on 0.5 for which the special cases allowing them to pass have been removed. In 0.6 and future releases those special cases are not necessary, and so I feel it is unnecessary to keep special cases around for those obscure cases.
The eleventh commit disables the self-lint test because of #200, and fixes a test hang on 0.6.
All of these issues are fixed, and test has been added to test/bugs.jl:
Fixes #56
Fixes #79
Fixes #81
Fixes #84
Fixes #88
Fixes #122
Fixes #167 [test is in test/doc.jl]
Fixes #195
I checked several old issues to see if they were fixed, and these ones were, but probably not by this PR (likely some intervening change fixed them). Tests have been added (see test/bugs.jl) to confirm that the bugs are fixed. I include them here because this PR tests for them, and because GitHub should close them when this is merged.
Closes #135