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

Remove VERSION constant from crystal init #15183

Open
BigBoyBarney opened this issue Nov 11, 2024 · 6 comments
Open

Remove VERSION constant from crystal init #15183

BigBoyBarney opened this issue Nov 11, 2024 · 6 comments

Comments

@BigBoyBarney
Copy link
Contributor

BigBoyBarney commented Nov 11, 2024

Following up on #15182.

It is apparently agreed upon, that crystal init should not presume that shards is available, which is silly IMO, but that's not what this issue is about.

If the automatically generated version constant cannot / should not infer the version number from shard.yml, I think it should be removed altogether.

Reason:

crystal init is one of the first things someone does when starting out with the language, and having a version constant in the generated source code file, gives the impression that it's the version of the shard they are editing, even though it's not. shard.yml is the source of the shard version number, so this can lead to misunderstandings and in unfortunate cases, mismatches.

@straight-shoota
Copy link
Member

I'm in favour of dropping the VERSION constant.
As it currently stands, it's probably causing more confusion and friction (sync with shard.yml) than being helpful.

Also not every Crystal project may even have a realistic use case for a version number embedded in the source code. It might be a good practice to have it available in general, but I feel the maintenance is too cumbersome.

@BigBoyBarney
Copy link
Contributor Author

From my personal experience, the first thing I do after running crystal init is either remove that line or replace it with the macro that gets the version from shard.yml.

I still think that overall it would be convenient and good to have a LibaryName.VERSION that one could call for every dependency, which initialising a version constant would promote, but the current implementation has more downsides than upsides.

In a previous PR, there was also talk of a parsing shard.yml directly, but nobody really responded to that suggestion. What are the current thoughts on that, 6 years later?

@straight-shoota
Copy link
Member

Another alterantive approach could be a mechanism to easily update the version and keep it in sync across multiple locations (e.g. something like shards release, crystal-lang/shards#98). But this feels like a big rabbit hole.

@straight-shoota
Copy link
Member

It is apparently agreed upon, that crystal init should not presume that shards is available

For reference: The macro command running shards means shards must to be available for building the source code. And that's something we should not assume. shards is commonly used to manage Crystal dependencies, but it's not part of the compiler and not an essential prerequisite for using Crystal.
That's why building a Crystal program should not depend on shards.

@Sija
Copy link
Contributor

Sija commented Nov 11, 2024

tbh, I cannot remember any situation where I'd be building Crystal code without shards available. Just my 2c

@BigBoyBarney
Copy link
Contributor Author

BigBoyBarney commented Nov 11, 2024

How about something like this?

{% if `which shards >/dev/null 2>&1 && echo found || echo not_found`.chomp == "found" %}
  VERSION = {{ `shards version`.chomp.stringify }}
{% end %}

It's a bit unwieldy, but could easily be adapted for Windows and macOS too.

Or the makeshift YAML parser you yourself proposed?

File.read_lines("#{__DIR__}/../shard.yml").find(&.starts_with?("version:")).try &.byte_slice(8).partition('#')[0].delete(%("')).strip

It got 2 thumbs downs, but nobody explained their issues with it.

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

No branches or pull requests

3 participants