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

Update to new julia package manager #6

Merged
merged 4 commits into from
Jan 11, 2019

Conversation

alblaz
Copy link
Contributor

@alblaz alblaz commented Jan 10, 2019

I would like to update the package so that a user could just type ]add https://github.com/CoolProp/CoolProp.jl and have a working version of CoolProp on julia.

In this PR I updated CoolProp.jl to utilize the files Project.toml and Manifest.toml to list all the dependencies of the package. This is the new standard in julia v1. The new package manager bundled with julia 1.x helps in the automatic generation of these files.

Yet, using the new package manager was not sufficient to run successfully ]add https://github.com/alblaz/CoolProp.jl#update-to-julia-v1.0.
and I got an error in the build process related to the line
const branchname = LibGit2.branch(LibGit2.GitRepo(abspath(@__FILE__, "..", "..")));

In order to get rid of this error I had to force branchname = "master" in build.jl . This is not an ideal fix and I think build.jl should be changed somehow. Yet now it is possible to run ]add https://github.com/alblaz/CoolProp.jl#update-to-julia-v1.0 and build a working CoolProp.jl.

Let me know your thoughts and your suggestions to modify build.jl,
Alberto

P.S. I noticed that 2 tests are failing for some reason. The tests are at line 10 and 11 in TestThrows.jl. I commented out these tests temporarily to check that everything else is working.

@ibell
Copy link
Contributor

ibell commented Jan 11, 2019

Is this ready to merge? I think it seems ok, but I'm no Julia expert

@alblaz
Copy link
Contributor Author

alblaz commented Jan 11, 2019

It is possible to merge. It would be good to figure out why the two tests that I commented out are failing but I don't think this should be blocking.

Regarding the build.jl file, right now it downloads only the latest version of CoolProp. I don't think this is a problem but the previous version could download either "latest" or "nightly" versions based on the name of the git branch. This implementation does not work anymore with the new package manager. The new package manager enables to download a package as a developer (]dev https://github.com/CoolProp/CoolProp.jl) or as a user (]add https://github.com/CoolProp/CoolProp.jl).
]dev downloads the whole git repository and is intended for people who wants to modify the original package while ]add downloads only a particular commit (default is the tagged version) and is intended for people who only want to use the package. The bottom line is that there is no git repository locally when performing a ]add https://github.com/CoolProp/CoolProp.jl hence it is not possible to use infos from git branch to choose the URL for the download.
This is probably not a problem and perhaps it is enough to be able to download the "latest" version.

@ibell
Copy link
Contributor

ibell commented Jan 11, 2019

I think it is fine to pull from master in general, but it would be nice to have flexibility in that.

@ibell ibell merged commit 691c3fb into CoolProp:master Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants