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

Move script files to bin + documentation #2971

Merged
merged 5 commits into from
Nov 26, 2018

Conversation

elia
Copy link
Member

@elia elia commented Nov 23, 2018

  • ➡️ Using bin/ for internal utils is the default for both Rails and Bundler.
    The script has now no extension and is marked as executable improving its
    usability (the user doesn't need to know how to deal with each extension) and
    maintainability (it's easy to switch to a ruby script just changing the
    shebang line).
  • 📝 The doc is updated and env variables are passed with env making life easier
    for some non-bash shells such as fish-shell.
  • 💁 Added some hints to the README on how to fix common setup issues.

@peterberkenbosch
Copy link
Contributor

nice.

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Thanks! Works great.

Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

I left one tiny comment, otherwise it looks great!

README.md Outdated Show resolved Hide resolved
Using bin/ for internal utils is the default for both Rails and Bundler.
The script has now no extension and is marked as executable improving its
usability (the user doesn't need to know how to deal with each extension) and
maintainability (it's easy to switch to a ruby script just changing the
shebang line).

The doc is updated and env variables are passed with `env` making life easier
for some non-bash shells such as fish-shell.
To help newcomers with the first run.
Many installation will only have a password-less root user available
(e.g. Homebrew).
@elia elia force-pushed the elia/move-script-files-to-bin branch from 13e68d4 to 0b3409e Compare November 23, 2018 22:56
Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

Looks great @elia! Do you mind squashing your commits?

@tvdeyen
Copy link
Member

tvdeyen commented Nov 24, 2018

@jacobherrington @elia the commits are great. There is no need to squash them.

Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

I'm alright with not squashing if you think they are okay @tvdeyen. I didn't realize that the last two were for PG and MySQL respectively.

@kennyadsl kennyadsl merged commit 8c6b41c into solidusio:master Nov 26, 2018
@kennyadsl kennyadsl deleted the elia/move-script-files-to-bin branch November 26, 2018 08:32
@kennyadsl
Copy link
Member

Thanks @elia !

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

Successfully merging this pull request may close these issues.

5 participants