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.

Sign up for GitHub

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

Jump to bottom

switch to new registrator and drop CI support for julia 0.7 #8

Merged
merged 2 commits into from
Apr 28, 2019
Merged

switch to new registrator and drop CI support for julia 0.7 #8

merged 2 commits into from
Apr 28, 2019

Conversation

johnnychen94
Copy link
Member

more details see JuliaImages/Images.jl#798

@johnnychen94 johnnychen94 mentioned this pull request Apr 21, 2019
Closed
Evizero
Evizero reviewed Apr 23, 2019
View reviewed changes
Project.toml Outdated
[extras]
ImageMagick = "6218d12a-5da1-5696-b52f-db25d2ecc6d1"
PaddedViews = "5432bcbf-9aad-5242-b902-cca2824c8663"
QuartzImageIO = "dca85d43-d64c-5e67-8c65-017450d5d020"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am guessing the @osx marcos etc are gone? should we maybe just stcik with ImageMagick in this case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to JuliaLang/Pkg.jl#165 this seems not implemented yet.

Evizero
Evizero approved these changes Apr 23, 2019
View reviewed changes
Copy link
Member

@Evizero Evizero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! thanks

@johnnychen94 johnnychen94 mentioned this pull request Apr 25, 2019
Merged
timholy
timholy reviewed Apr 25, 2019
View reviewed changes
Project.toml Outdated Show resolved Hide resolved
@johnnychen94
Copy link
Member Author

Is this problem caused by https://github.com/JuliaIO/ImageMagick.jl/blob/40d725a1f0bf9051585df2b0a9bf62a579c9955f/REQUIRE#L6?

Perhaps we need to bring back 0.7 compatibility

@johnnychen94
Copy link
Member Author

johnnychen94 commented Apr 25, 2019 •
edited
Loading

Looks like either ImageMagick or ImageShow needs a version bump first to solve the version conflict, regardless of the CI failure.

@johnnychen94
add Project.toml and delete REQUIRE
bf03f43 
version 0.1.3 as a temporary version that resolve the version
conflict with ImageMagick by restricting ImageCore version while
switching to the new registrator system
@codecov
Copy link

codecov bot commented Apr 26, 2019

Codecov Report

Merging #8 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master       #8   +/-   ##
=======================================
  Coverage   62.33%   62.33%           
=======================================
  Files           2        2           
  Lines          77       77           
=======================================
  Hits           48       48           
  Misses         29       29

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8db749...bf03f43. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 26, 2019 •
edited
Loading

Codecov Report

Merging #8 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master       #8   +/-   ##
=======================================
  Coverage   62.33%   62.33%           
=======================================
  Files           2        2           
  Lines          77       77           
=======================================
  Hits           48       48           
  Misses         29       29

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8db749...48a800d. Read the comment docs.

@johnnychen94
Copy link
Member Author

johnnychen94 commented Apr 26, 2019 •
edited
Loading

I find two solutions that might help

Option1:

Adding a compatibility restriction to ImageCore = "< 0.8" solves the version conflict

  1. bump to v0.1.3 as a temporary/transitory version, I think that can also make Move to Project.toml and update Travis JuliaIO/ImageMagick.jl#152 work, perhaps there should be a similar operation on ImageMagick side cc: @ianshmean

  2. When the conflict resolved, drop the ImageCore restriction and bump to 0.2.0

Still keep compatibility to julia 0.7 until ImageShow v0.2.0

Option2:

drop the ImageCore restrction, merge and bump the version to 0.2.0 regardless of the CI failure.

@johnnychen94
Copy link
Member Author

ping @timholy

@IanButterworth
Copy link
Member

JuliaIO/ImageMagick.jl#152 is now merged and released. Awaiting merging on General

@johnnychen94
Copy link
Member Author

johnnychen94 commented Apr 27, 2019 •
edited
Loading

Just noticed it, thanks!
In that case ImageShow can directly upgrade to 0.2.0

@johnnychen94 johnnychen94 mentioned this pull request Apr 27, 2019
Closed
@timholy timholy closed this Apr 27, 2019
@timholy timholy reopened this Apr 27, 2019
@timholy timholy mentioned this pull request Apr 27, 2019
Merged
@RalphAS RalphAS closed this Apr 28, 2019
@RalphAS RalphAS reopened this Apr 28, 2019
@RalphAS
Copy link
Collaborator

RalphAS commented Apr 28, 2019

Fixed by above-mentioned PR to General.

@RalphAS RalphAS merged commit d9843bc into JuliaImages:master Apr 28, 2019
@johnnychen94 johnnychen94 deleted the new-registrator branch April 29, 2019 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timholy timholy timholy left review comments

@Evizero Evizero Evizero approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@johnnychen94 @IanButterworth @RalphAS @timholy @Evizero