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

Jump to bottom

Show the shard's name when running scripts #326

Merged
merged 2 commits into from
Mar 11, 2020
Merged

Show the shard's name when running scripts #326

merged 2 commits into from
Mar 11, 2020

Conversation

bcardiff
Copy link
Member

@bcardiff bcardiff commented Mar 9, 2020

When running $ shards install it will now append the shard name where the script is defined

For example when running it in lucky it will show the precompiled_tasks for avram and make bin from ameba are run.

... stripped ...
Installing teeplate (0.8.0)
Installing blank (0.1.0)
Installing db (0.8.0)
Installing pg (0.20.0)
Postinstall script/precompile_tasks (avram)
Postinstall make bin (ameba)
Writing shard.lock

When the command fails, only the command is displayed with no path/dependency name.

@straight-shoota
Copy link
Member

Maybe a more logical ordering would be to put the dependency name before the command? Postinstall of avram: script/precompile_tasks. This would also help to keep the names aligned in case the command wraps.

When the command fails, only the command is displayed with no path/dependency name.

Isn't the shard name a significant information in case of a failure? I'd expect that to show when the command fails as well.

@bcardiff
Copy link
Member Author

Feedback addressed @straight-shoota

straight-shoota
straight-shoota approved these changes Mar 10, 2020
View reviewed changes
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

Forwarding script and dependency to the run method doesn't seem ideal considering encapsulation, but I guess it is okayish.

@bcardiff bcardiff merged commit 1c648e5 into crystal-lang:master Mar 11, 2020
@bcardiff bcardiff deleted the feature/show-name-in-scripts branch March 11, 2020 21:46
@bcardiff bcardiff added this to the v0.10.0 milestone Mar 27, 2020
taylor pushed a commit to vulk/shards that referenced this pull request Aug 11, 2020
@taylor
Show the shard's name when running scripts (crystal-lang#326)
233af25 
* Show the shard's name when running scripts

* Add script and dependency name on failure. Add specs
f-fr pushed a commit to f-fr/shards that referenced this pull request Jan 2, 2021
Show the shard's name when running scripts (crystal-lang#326)
c5ed1ff 
* Show the shard's name when running scripts

* Add script and dependency name on failure. Add specs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@straight-shoota straight-shoota straight-shoota approved these changes

Assignees
No one assigned
Labels
kind:feature
Projects
None yet
Milestone
v0.10.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@bcardiff @straight-shoota