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

Make Pkg.update() return nothing #16690

Merged
merged 1 commit into from
Jun 1, 2016
Merged

Conversation

zhmz90
Copy link
Contributor

@zhmz90 zhmz90 commented Jun 1, 2016

When Pkg.update() finishes with no error, false is returned which can confuse users.

Current state is:

INFO: Updating OpenGene master... b28b0d93 → c3139d86
INFO: Computing changes...
INFO: Removing BufferedStreams v0.1.1
INFO: Removing Libz v0.1.1
false

julia> 

When `Pkg.update()` finishes with no error, `false` is returned which can confuse users.
@JeffBezanson JeffBezanson merged commit f0a8a55 into JuliaLang:master Jun 1, 2016
@tkelman
Copy link
Contributor

tkelman commented Jun 1, 2016

should probably test this

@tkelman tkelman added the needs tests Unit tests are required for this change label Jun 1, 2016
@zhmz90
Copy link
Contributor Author

zhmz90 commented Jun 1, 2016

Ok, I will make tests later.

@tkelman
Copy link
Contributor

tkelman commented Jun 1, 2016

there are already a few calls to Pkg.update during test/pkg.jl you could just add checks to:

julia/test/pkg.jl

Lines 186 to 205 in e56c2a1

Pkg.update()
Pkg.installed()["Example"] == v"0.4.0"
end
# add a directory that is not a git repository
begin
mkdir(joinpath(Pkg.dir(), "NOTGIT"))
Pkg.installed("NOTGIT") == typemin(VersionNumber)
Pkg.installed()["NOTGIT"] == typemin(VersionNumber)
end
begin
# don't bork when a Pkg repo is bare, issue #13804
pth = joinpath(Pkg.dir(), "BAREGIT")
mkdir(pth)
# create a bare repo (isbare = true)
repo = LibGit2.init(pth, true)
@test repo.ptr != C_NULL
finalize(repo)
Pkg.update()

zhmz90 pushed a commit to zhmz90/julia that referenced this pull request Jun 1, 2016
Add a test for Pkg.update() to check whether the return value is nothing. Related to my previous PR JuliaLang#16690.
@zhmz90
Copy link
Contributor Author

zhmz90 commented Jun 1, 2016

@tkelman Thanks for your helpful instructions, I have made a new PR now.

@tkelman tkelman removed the needs tests Unit tests are required for this change label Jun 1, 2016
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.

3 participants