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

Change Installation section of README template to numbered list #6942

Merged
merged 5 commits into from
Oct 18, 2018

Conversation

wooster0
Copy link
Contributor

@wooster0 wooster0 commented Oct 14, 2018

Follow-up of #6914

I didn't put a dot behind step 2 for consistency because there aren't dots either behind the steps in the Contributing section.

So now the Installation section would look like this:


Installation

  1. Add the dependency to your shard.yml:
dependencies:
  library:
    github: user/library
  1. Run shards install

@@ -5,15 +5,15 @@ TODO: Write a description here
## Installation

<%- if config.skeleton_type == "lib" -%>
Add this to your application's `shard.yml`:
1. Run `shards init`
2. Add this to your application's `shard.yml`:
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to detail what "this" is:

Add the dependency to your application's shard.yml

Copy link
Contributor Author

@wooster0 wooster0 Oct 15, 2018

Choose a reason for hiding this comment

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

Let's exclude "application's". Because a shard.yml doesn't necessarily has to be used for just one application, it can be used for multiple applications.

So "Add the dependency to your shard.yml:"

@RX14
Copy link
Contributor

RX14 commented Oct 15, 2018

In fact, most of the time you don't have to run shards init, because you've used other shards before or you've used crystal init. So I don't think we should include this advice.

@wooster0
Copy link
Contributor Author

Most of the time, but not always. I think the section shouldn't be incomplete and describe all steps to install the shard. But I agree, most of the time shard.yml is probably there.
Maybe "Run shards init unless shard.yml already exists" or something similar?

@RX14
Copy link
Contributor

RX14 commented Oct 16, 2018

I don't think we should assume that people don't know to use shards, there's obviously not enough space to explain the whole process of installing a shard.

@asterite
Copy link
Member

Looking at one Ruby gem they say:

  1. Add something to the Gemfile
  2. Run bundle
  3. (or install it with the gem command)

I think we just need to say:

  1. Add to shards.yml
  2. Execute shards

Nothing else. shards init is for initializing a project, and in fact if you use crystal init it will be there already.

spec/compiler/crystal/tools/init_spec.cr Outdated Show resolved Hide resolved
```yaml
dependencies:
<%= config.name %>:
github: <%= config.github_name %>/<%= config.name %>
```
3. Run `shards install`
2. Run `shards`
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

Copy link
Member

@sdogruyol sdogruyol left a comment

Choose a reason for hiding this comment

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

Thak you @r00ster91 👍

@sdogruyol sdogruyol merged commit e495121 into crystal-lang:master Oct 18, 2018
@sdogruyol sdogruyol added this to the 0.27.0 milestone Oct 18, 2018
@Sija
Copy link
Contributor

Sija commented Oct 19, 2018

@sdogruyol It seems since sometime you stopped squashing commits, which leads to pretty messy commit list, is it intentional?

@sdogruyol
Copy link
Member

@Sija nope it's not intentional. I just hate Github for changing my default merge strategy based on other repositories. I think eventually I need to write a Chrome plugin to avoid that.

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.

8 participants