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

Tidyup patch #191

Merged
merged 10 commits into from Jan 9, 2020
Merged

Tidyup patch #191

merged 10 commits into from Jan 9, 2020

Conversation

@aminya
Copy link
Contributor

aminya commented Jan 8, 2020

Based on the results of the tests. Now, most of the tests pass nicely.

I dropped Julia 1.0 because MbelTLS 0.7 only supports Julia 1.2 and above. If you think MbelTLS compat can be relaxed (like using older versions), we can support Julia 1.0 again.

aminya added 10 commits Jan 8, 2020
Manifest should only be used to use a non-registered package as the 
dependency.
@aminya

This comment has been minimized.

Copy link
Contributor Author

aminya commented Jan 8, 2020

Windows tests fail. I think the error requires technical knowledge of Genie internals.
Some temp files are missing:
https://github.com/GenieFramework/Genie.jl/pull/191/checks?check_run_id=379172404#step:5:50

@essenciary

This comment has been minimized.

Copy link
Member

essenciary commented Jan 9, 2020

Same for Julia 1.0 - looks like a dependency issue. Maybe we should just remove tests for anything under 1.3?

I'll dig into the windows issue.

@aminya

This comment has been minimized.

Copy link
Contributor Author

aminya commented Jan 9, 2020

Same for Julia 1.0 - looks like a dependency issue. Maybe we should just remove tests for anything under 1.3?

Yes, tests are disabled. Now we only test on 1.3 and nightly. Although we support 1.2 too.

If you are determined to support Julia 1.0, you should edit the compat line for MbedTLS to this:

MbedTLS = "0.6.7, 0.7"

This will relax its version requirement allowing to support Julia 1.0 again. You should test it yourself to check if it actually works though.

@aminya

This comment has been minimized.

Copy link
Contributor Author

aminya commented Jan 9, 2020

Also, I recommend removing Manifest.toml in the master to resolve the conflict. You shouldn't commit Manifest unless you want to add a specific unregistered package as your dependency (which you don't have any).

@essenciary

This comment has been minimized.

Copy link
Member

essenciary commented Jan 9, 2020

@aminya Thanks for the tips! Sorry, I didn't notice you submitted a PR for the changes. I think not testing on Julia 1.0 is fine, I'm not committed to explicitly supporting it anyway. I'll merge.

Re Manifest.toml I agree. I'll remove.

@essenciary essenciary merged commit b744fa3 into GenieFramework:master Jan 9, 2020
5 of 7 checks passed
5 of 7 checks passed
Julia 1.3 - ubuntu-latest - x64
Details
Julia 1.3 - macOS-latest - x64
Details
Julia 1.3 - windows-latest - x64 Julia 1.3 - windows-latest - x64
Details
Julia nightly - ubuntu-latest - x64
Details
Julia nightly - macOS-latest - x64
Details
Julia nightly - windows-latest - x64 Julia nightly - windows-latest - x64
Details
Documentation
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.