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

New package: mlpack v3.2.3 #9767

Merged
merged 1 commit into from
Mar 1, 2020
Merged

Conversation

JuliaRegistrator
Copy link
Contributor

@JuliaRegistrator JuliaRegistrator commented Feb 19, 2020

Initial release of mlpack's Julia bindings.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 19, 2020

Your new package pull request does not meet the following guidelines for auto-merging:

  • Name does not meet all of the following: starts with an uppercase letter, ASCII alphanumerics only, ends in a lowercase letter. Important note: It is okay to have a package name that ends in an uppercase letter. However, you will need to wait until a registry maintainer manually approves the name.
  • Version number is not 0.0.1, 0.1.0, or 1.0.0

Note that the guidelines are only required for the pull request to be merged automatically. However, it is strongly recommended to follow them, since otherwise the pull request needs to be manually reviewed and merged by a human.

Since you are registering a new package, please make sure that you have read the package naming guidelines: https://julialang.github.io/Pkg.jl/dev/creating-packages/#Package-naming-guidelines-1


If you want to prevent this pull request from being auto-merged, simply leave a comment. If you want to post a comment without blocking auto-merging, you must include the text [noblock] in your comment.

@rcurtin
Copy link

rcurtin commented Feb 19, 2020

Right, we've broken the guidelines. But I think we have good reason. :)

  • The name matches the upstream mlpack package: https://github.com/mlpack/mlpack, where it's mlpack and not Mlpack.
  • The version is 3.2.3, since that's the current stable version of mlpack, and the first version that has Julia bindings. So we're not starting from 0.x (that was 2007 :)).

I believe I've solved the compat issue, but I'm not sure how to get @JuliaRegistrator to rebuild the package.

Copy link
Member

@ararslan ararslan left a comment

Choose a reason for hiding this comment

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

I don't think this should be merged with the current name. To quote @davidanthoff in Slack:

There are lots of Julia packages that wrap some upstream package that uses some capitalization that doesn't conform with the Julia norm, and the practice for the Julia package has been to adhere to the Julia convention.
So I don't see a reason to make an exception here, to be honest 🙂

That summarizes my thoughts here exactly.

@StefanKarpinski
Copy link
Contributor

It seems perfectly fine to me for the name of a JLL to match the name of the library that it wraps. That's exactly what I did with bsdiff.

@rcurtin
Copy link

rcurtin commented Feb 28, 2020

Thanks to @ericphanson in mlpack/mlpack.jl#2, the upper bound on the compat entry is added now. I'm not sure if I need to do anything to make an automatic rebuild happen, but I'm happy to handle anything else if needed. 👍

@DilumAluthge
Copy link
Member

Retrigger Registrator. This will update the PR.

@ararslan
Copy link
Member

It seems perfectly fine to me for the name of a JLL to match the name of the library that it wraps

This isn't a JLL registration

@rcurtin
Copy link

rcurtin commented Feb 28, 2020

@DilumAluthge I can see how to retrigger if I used the GitHub App, but I used the web registrator at https://pkg.julialang.org/registrator/, so the instructions here don't seem to apply. Does the web registrator automatically update a PR if one is already open? I don't want to accidentally open a different one if I don't need to. 👍

@DilumAluthge
Copy link
Member

Yeah it should update this PR. Try it out. If it makes a duplicate for some reason, we can close one of the PRs, no big deal.

@rcurtin
Copy link

rcurtin commented Feb 28, 2020

Victory, thank you @DilumAluthge! 👍

UUID: cf18a64e-e90e-11e8-37b7-fb5df6478bc0
Repo: https://github.com/mlpack/mlpack.jl.git
Tree: 7844ffb9a515fac525eba2ffe01bf45b90ff1d0d

Registrator tree SHA: f50e50c1d2a1b9694b1d5749fdb25fef2ca4c291
@rcurtin
Copy link

rcurtin commented Feb 28, 2020

Alright, looks like everything is okay after the rebuild. 👍

@fredrikekre
Copy link
Member

It seems perfectly fine to me for the name of a JLL to match the name of the library that it wraps. That's exactly what I did with bsdiff.

As Alex pointed out, this is a regular Julia package (JLLs are as well but..), and bsdiff is in fact called bsdiff_jll, just like the mlpack entry in Yggdrasil is called mlpack_jll.

@StefanKarpinski
Copy link
Contributor

Ultimately the choice of package name is up to the author. We can make recommendations, but as long as they pick a name that isn't clearly typosquatting or offensive, it's up to them.

@DilumAluthge
Copy link
Member

Ultimately the choice of package name is up to the author. We can make recommendations, but as long as they pick a name that isn't clearly typosquatting or offensive, it's up to them.

It seems clear to me that the author wants this name, and neither of the two criteria in Stefan’s comment are met.

@DilumAluthge DilumAluthge merged commit 5b2499a into master Mar 1, 2020
@DilumAluthge DilumAluthge deleted the registrator/mlpack/cf18a64e/v3.2.3 branch March 1, 2020 13:26
@rcurtin
Copy link

rcurtin commented Mar 1, 2020

Thanks all, much appreciated. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants