-
Notifications
You must be signed in to change notification settings - Fork 571
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
Add Muesli library #10341
base: master
Are you sure you want to change the base?
Add Muesli library #10341
Conversation
I've left a few comments. From my point of view most of these are stylistic changes. |
@eschnett Thanks for the review comments, I addressed them all. |
M/muesli/build_tarballs.jl
Outdated
platforms = filter(!Sys.iswindows, supported_platforms()) | ||
platforms = filter(!Sys.isapple, platforms) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why removing these two?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ll try to get this running for all systems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done :)
Co-authored-by: Mosè Giordano <[email protected]>
M/muesli/build_tarballs.jl
Outdated
# `julia build_tarballs.jl --help` to see a usage message. | ||
using BinaryBuilder, Pkg | ||
|
||
name = "muesli" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like there are two "muesli" libraries out there, which both seem like plausible candidates for the Muesli name in the Julia General registry:
- https://materials.imdea.org/muesli/ , i.e., https://bitbucket.org/ignromero/muesli
- https://perso.univ-rennes1.fr/edouard.canot/muesli/
Is there an argument for https://materials.imdea.org/muesli being more "Muesli" than the other Muesli?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's also https://github.com/bmwcarit/muesli, and https://muesli.uni-muenster.de, but it's quite easy to argue that those two could use longer names, e.g., MuesliSerializationLibrary and, MuensterSkeletonLibrary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats a really good point I havn't thought about yet. For me it would be fine if we name it MuesliMaterials
. That would also reflect for example the contact email address ([email protected])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MuesliMaterials sounds fine - I don't know any of the Muesli libraries, so whatever name makes the most sense is fine (also "Muesli", if Muesli is more Muesli than the other mueslies :-).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha its just a super generic name I'd argue. I will stick with MuesliMaterials
, then people who don't know the library can get atleast an idea what its about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good :-)
Please don't merge yet, I want to do some testing before locally.