Skip to content
This repository has been archived by the owner on Oct 21, 2021. It is now read-only.

Fixes for v0.5 #228

Merged
merged 7 commits into from
Aug 27, 2016
Merged

Fixes for v0.5 #228

merged 7 commits into from
Aug 27, 2016

Conversation

ranjanan
Copy link
Collaborator

This attempts to fix Graphs.jl for v0.5

@@ -19,13 +19,13 @@ vertex_index(v::KeyVertex) = v.index

type ExVertex
index::Int
label::UTF8String
label::String
Copy link

@KristofferC KristofferC Aug 16, 2016

Choose a reason for hiding this comment

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

Maybe Compat.UTF8String in fields of types if we still want them to be a concrete types.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Otherwise this changes the API and performance on 0.4.

@codecov-io
Copy link

codecov-io commented Aug 16, 2016

Current coverage is 85.65% (diff: 85.71%)

Merging #228 into master will decrease coverage by 0.05%

@@             master       #228   diff @@
==========================================
  Files            24         24          
  Lines          1435       1443     +8   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1230       1236     +6   
- Misses          205        207     +2   
  Partials          0          0          

Powered by Codecov. Last update 12511ed...e206363

@@ -63,15 +63,15 @@ function to_dot(attrs::AttributeDict)
if isempty(attrs)
""
else
string("[",join(map(a -> to_dot(a[1], a[2]), attrs),","),"]")
string("[",join(map(a -> to_dot(a[1], a[2]), [attrs...]),","),"]")
Copy link
Contributor

Choose a reason for hiding this comment

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

would collect work for this?

Copy link
Collaborator Author

@ranjanan ranjanan Aug 16, 2016

Choose a reason for hiding this comment

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

collect(attrs) instead of [attrs...] works too.

@@ -3,8 +3,8 @@ os:
- linux
- osx
julia:
- 0.3
Copy link
Contributor

Choose a reason for hiding this comment

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

don't delete this unless you also change the minimum julia version in REQUIRE

@ViralBShah
Copy link

Thanks @ranjanan

@@ -179,7 +179,7 @@ edge_property{T,V}(visitor::VectorEdgePropertyInspector{T}, e, g::AbstractGraph{
edge_property_requirement{T, V}(visitor::AbstractEdgePropertyInspector{T}, g::AbstractGraph{V}) = @graph_requires g edge_map

type AttributeEdgePropertyInspector{T} <: AbstractEdgePropertyInspector{T}
attribute::UTF8String
attribute::String
Copy link
Contributor

Choose a reason for hiding this comment

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

also Compat.UTF8String

@tkelman tkelman mentioned this pull request Aug 25, 2016
@tkelman tkelman merged commit a718b8c into JuliaAttic:master Aug 27, 2016
@ViralBShah
Copy link

@tkelman Should we move Graphs.jl to a new org and then tag a new release?

@tkelman
Copy link
Contributor

tkelman commented Aug 27, 2016

which org? JuliaArchive or something else?

@ViralBShah
Copy link

JuliaArchive sounds good.

@ViralBShah
Copy link

Should we also rename it to LegacyGraphs in the process?

@tkelman
Copy link
Contributor

tkelman commented Aug 27, 2016

Renaming basically means re-registering a new copy from scratch. As described in #227 (comment), it's not worth doing that unless we do a merge commit and replace the content with something new.

@ViralBShah
Copy link

Ok, let's just move it.

@tkelman
Copy link
Contributor

tkelman commented Aug 27, 2016

Moved, but we should document the fact that this isn't actively maintained. #227 (comment)

@ViralBShah
Copy link

Can you add a note to the README?

@ViralBShah
Copy link

Although isn't being in JuliaArchives enough of a signal?

@tkelman
Copy link
Contributor

tkelman commented Aug 27, 2016

JuliaArchive doesn't have any description of what it means for a package to be here. You also only briefly see what user/organization a package comes from when you do Pkg.add.

@ViralBShah
Copy link

Is README enough?

@ranjanan Can you also register the new version in METADATA?

@tkelman
Copy link
Contributor

tkelman commented Aug 27, 2016

Remember to do Pkg.tag("Graphs", :minor) since support for Julia 0.3 was dropped.

@ranjanan ranjanan deleted the RA/fix branch August 27, 2016 14:08
@ranjanan
Copy link
Collaborator Author

Sure, will tag a new version once #229 is merged.

@ranjanan
Copy link
Collaborator Author

Tagged. (JuliaLang/METADATA.jl#6148)

KristofferC pushed a commit to KristofferC/Graphs.jl that referenced this pull request Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants