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

SPEC: Allow periods in shard name #321

Closed
wants to merge 1 commit into from

Conversation

straight-shoota
Copy link
Member

This is a proposal to allow period (.) in the name of a spec.

The main reason to allow this is because its already in use (since there is no strict enforcement anyways). There are a number of shards ending with .cr, for example https://github.com/TPei/progress_bar.cr, https://github.com/krthr/haye.cr, https://github.com/imdrasil/hermes.cr
This is somewhat of an attempt to "legalize" those names.

I'm not aware of any specific reason for prohibiting this character. There might be some source of confusion with respect to file extensions. But since shard names are typically used as directories, I don't see any real issues there.
The general idea is to keep the allowed character limited, but basic special characters should be allowed (dashes and underscores are already allowed).

/cc crystal-lang/crystal#8737

@ysbaddaden
Copy link
Contributor

I don't see any benefit for allowing dots in shard names. I see confusion, especially when the shard name ends with .cr that is similar to having crystal appear in your shard name, which we already recommend against.

@Sija
Copy link
Contributor

Sija commented Feb 7, 2020

@straight-shoota IMO the repo name and shard name are two different things - there's nothing wrong in having a . within the repo name but in the shard name...?

@straight-shoota
Copy link
Member Author

@Sija It's not just the repo names. The referenced repos define the shard name ending with .cr in their shard.yml.

@Sija
Copy link
Contributor

Sija commented Feb 7, 2020

IMO that's a bit too much. Imagine require "foo.cr"...

@straight-shoota
Copy link
Member Author

Thanks for your input. There seems to be no support for periods in shard names.

We could consider taking measures to prohibit them altogether. That would however break existing shards with periods in their name and shards using them as dependencies.
On shardbox.org there are six shards with periods, all ending in .cr:

None of them is a dependency of any shard known on shardbox.org. So at least it wouldn't have a big impact if those ceased to work.

I'd definitely try to avoid getting new shards with invalid names. I'll update crystal init to properly validate the name. shards init already sanitizes the folder name.

@straight-shoota
Copy link
Member Author

shards validate would also notify about invalid shard name (#29).

Also should we maybe issue warnings when such a shard is installed, similar to what #341 added for mismatching tag and SPEC version?

@straight-shoota
Copy link
Member Author

Closing. Periods in shard names should not be support.
Warnings can be added when a validation command is introduced.

@straight-shoota straight-shoota deleted the feature/shard-name-period branch November 19, 2020 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants