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

Show errors when NPM install fails #127

Merged
merged 1 commit into from
Nov 7, 2023
Merged

Conversation

cllns
Copy link
Member

@cllns cllns commented Nov 7, 2023

Sample output now:

% exe/hanami new bookshelf
Created bookshelf/
-> Within bookshelf/
Created .gitignore
Created .env
Created README.md
Created Gemfile
Created Rakefile
Created Procfile.dev
Created config.ru
Created bin/dev
Created config/app.rb
Created config/settings.rb
Created config/routes.rb
Created config/puma.rb
Created lib/tasks/.keep
Created lib/bookshelf/types.rb
Created app/actions/.keep
Created app/action.rb
Created app/view.rb
Created app/views/helpers.rb
Created app/templates/layouts/app.html.erb
Created package.json
Created config/assets.mjs
Created app/assets/js/app.js
Created app/assets/css/app.css
Created app/assets/images/favicon.ico
Created public/404.html
Created public/500.html
Running Bundler install...
Running npm install...
NPM ERROR:
    npm ERR! code ETARGET
    npm ERR! notarget No matching version found for hanami-assets@^2.1.0-rc1.
    npm ERR! notarget In most cases you or one of your dependencies are requesting
    npm ERR! notarget a package version that doesn't exist.

    npm ERR! A complete log of this run can be found in:
    npm ERR!     /Users/sean/.npm/_logs/2023-11-07T01_38_25_312Z-debug-0.log
Running Hanami install...

I don't think we should fail when npm fails, but it would be nice to let the user know there was a problem. This could happen if there's some issue with their npm install, or if they don't have it installed at all.

Open to adding more text, or making this smaller (if it's a long error message, the whole thing will show). For example, we could just show the last line npm ERR! /Users/sean/.npm/_logs/2023-11-07T01_38_25_312Z-debug-0.log if we want to.

(The underlying issue here was fixed in #119, but I reintroduced it locally to get the failure)

@cllns cllns requested a review from timriley November 7, 2023 01:42
Copy link
Member

@timriley timriley left a comment

Choose a reason for hiding this comment

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

I think this is good to get in. This is something I would've attempted to do myself, but it didn't make the cut with everything else I was tidying in the last few days.

I think we can probably make the error output display even more nicely than here, but I reckon that's a nice-to-have and we can consider that when we look at the overall output of this hanami new command in general. What we have here is already better than what we had before.

@jodosha are you happy to bring this in? We can do it either now or before stable.

@timriley timriley requested a review from jodosha November 7, 2023 10:26
@timriley timriley added this to the v2.1.0 milestone Nov 7, 2023
Copy link
Member

@jodosha jodosha left a comment

Choose a reason for hiding this comment

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

@cllns Thanks for this enhancement. This is very good for troubleshooting. 👍

@jodosha jodosha added the enhancement New feature or request label Nov 7, 2023
@jodosha jodosha self-assigned this Nov 7, 2023
@jodosha jodosha merged commit 6a76576 into main Nov 7, 2023
@jodosha jodosha deleted the report-when-npm-install-fails branch November 7, 2023 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants