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.

Sign up for GitHub

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

Use shards version in crystal init tool #5428

Closed
wants to merge 1 commit into from
Closed

Use shards version in crystal init tool #5428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions spec/compiler/crystal/tools/init_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,18 +147,11 @@ dependencies:
end

describe_file "example/src/example.cr" do |example|
example.should eq(%{require "./example/*"

# TODO: Write documentation for `Example`
example.should eq(%{# TODO: Write documentation for `Example`
module Example
# TODO: Put your code here
end
})
end
VERSION = {{ `shards version \#{__DIR__}`.chomp.stringify }}

describe_file "example/src/example/version.cr" do |version|
version.should eq(%{module Example
VERSION = "0.1.0"
# TODO: Put your code here
end
})
end
Expand Down
1 change: 0 additions & 1 deletion src/compiler/crystal/tools/init.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ module Crystal
template ShardView, "shard.yml.ecr", "shard.yml"

template SrcExampleView, "example.cr.ecr", "src/#{config.name}.cr"
template SrcVersionView, "version.cr.ecr", "src/#{config.name}/version.cr"

template SpecHelperView, "spec_helper.cr.ecr", "spec/spec_helper.cr"
template SpecExampleView, "example_spec.cr.ecr", "spec/#{config.name}_spec.cr"
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/crystal/tools/init/template/example.cr.ecr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "./<%= config.name %>/*"

# TODO: Write documentation for `<%= module_name %>`
module <%= module_name %>
VERSION = {{ `shards version #{__DIR__}`.chomp.stringify }}
Copy link
Member

Choose a reason for hiding this comment

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

You can't just assume that shards is available, let alone in PATH. This defeats the whole point of decentralization. Do you really think Crystal libraries should literally not work if shards is not in PATH?

Copy link
Member

@oprypin oprypin Jan 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

In case it wasn't clear, this literally puts the line VERSION = {{ `shards version #{__DIR__}`.chomp.stringify }} into all new projects. It's not a dependency of crystal init, it's a dependency of every library.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not particularly fond of hard dependency on shards either, here it is IMHO justified.
crystal init is perhaps the first real compiler command run by novice developers following a Crystal tutorial. It should have all batteries included and provide an easy way into the Crystal ecosystem which includes shards.
The init command is opinionated but dedicated to provide an easy tool for developers to make them happy. If that doesn't fit with specific needs, don't use it. For the vast majority of users this is probably the best thing. And it's good for the ecosystem if people use shards so it doesn't hurt to advocate it.

Copy link
Member

Choose a reason for hiding this comment

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

In case it wasn't clear, this literally puts the line VERSION = {{ `shards version #{__DIR__}`.chomp.stringify }} into all new projects. It's not a dependency of crystal init, it's a dependency of every library.

Copy link
Member

Choose a reason for hiding this comment

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

I know, this is a considerable downside. Maybe a better long-term solution would be to include a simple parser for shards.yml in the standard library (maybe even directly as a macro method). I've already suggested that idea in #4754 to provide metadata about the project for the docs generator. This way it wouldn't depend on the executable but read shards.yml directly.


# TODO: Put your code here
end
3 changes: 0 additions & 3 deletions src/compiler/crystal/tools/init/template/version.cr.ecr
View file
Open in desktop

This file was deleted.