Remove the crystal lib generator code. #800
Conversation
… to generate on Windows. Fixes #741
| @@ -89,171 +82,80 @@ class LuckyCli::Generators::Web | |||
| if default_directory? | |||
| project_dir | |||
| else | |||
| [full_project_directory.chomp('/'), project_dir].join('/') | |||
| File.join(full_project_directory.chomp('/'), project_dir) | |||
There was a problem hiding this comment.
I'm not sure how this looks on Windows, by we may need to see if there's some sort of Crystal::System::FileSeparator or whatever. I'm assuming on Windows we'd need to chomp('\')...
There was a problem hiding this comment.
While you can use File methods for dealing with Paths, because they are just wrappers around Path, I'd recommend to just use Path directly instead. It makes it more clear what you're doing (e.g. path manipulation), and use File for actual filesystem operations (e.g. read/write a file).
You don't actually need #chomp here:
| File.join(full_project_directory.chomp('/'), project_dir) | |
| File.join(full_project_directory, project_dir) |
But this is my suggestion (which is the same as File#join, just prefer using Path):
| File.join(full_project_directory.chomp('/'), project_dir) | |
| Path.new(full_project_directory).join(project_dir).to_s |
However, my above suggestion would be better suited in a separate PR to change all other places using File instead of Path (I might add this to my lucky_template work).
There was a problem hiding this comment.
Yeah, I never thought about using Path until I started seeing your code. I think that makes more sense. Maybe I'll just merge this PR in as-is, then in another one we can go through it all and really start cleaning stuff up.
|
Looks like the ameba failure is actually caused from crystal-ameba/ameba#372 so it'll be fixed on the next release. Not sure why the sectester stuff is failing... Seems it can't connect? @bararchy do you have any ideas on https://github.com/luckyframework/lucky_cli/actions/runs/4973638473/jobs/8899605681 ? |
|
@jwoertink seems strange... do we maybe need to install |
Fixes #741
Ref luckyframework/lucky#1812
This PR removes the initial step of running
crystal init app .... We had been using this command to bootstrap the structure, but then remove almost every file after it was generated.This now generates an empty directory, and only inserts what is needed which, in theory, should reduce file operations, as well as help in the effort to running on Windows.
/cc @mdwagner