samueleaton / sentry

Build/Runs your crystal application, watches files, and rebuilds/restarts app on file changes
MIT License
286 stars 27 forks source link

Closes #20. Add `Sentry::Config` for managing configuration, and check `.sentry.yml` on startup #21

Closed faultyserver closed 6 years ago

faultyserver commented 6 years ago

This PR introduces Sentry::Config as a single object to manage the configuration of a sentry instance. The CLI also now checks for a .sentry.yml file in the project directory to load configurations automatically. These configurations can still be overridden by specifying options the command line, though this is now entirely optional, as all options are supported from the YAML file.

An example and explanation of the .sentry.yml format can be found in .sentry.example.yml.

samueleaton commented 6 years ago

very quick turnaround. Do you think it would be worth allowing users to specify their own path to the config file (e.g. -c, --config) like I mentioned in the Issue?

And could you run crystal tool format on src directory? :)

faultyserver commented 6 years ago

I knew I forgot something! Coming right up.

faultyserver commented 6 years ago

I'm not entirely sure about the semantics at the end of sentry_cli.cr. It's a little different than before, but I think it's safer now?

Before, if the user specified a name with --name, but didn't have a name from shard.yml and didn't set a custom build command with --build, sentry would start the process runner anyway (because the local process_name variable was being re-used).

Now, it will fail if the name isn't set in shard.yml, no matter what. Since the display name doesn't affect the default build/run commands, I think this makes sense.

However, if the user sets custom build/run commands, it should be fine that they don't have a name in shard.yml.

Not sure how to go about doing that, or if it's worthwhile for what I think would be a very minor edge case.

faultyserver commented 6 years ago

Quick bump.

I've been using this pretty regularly for the past 2 weeks with no issues, so I think it's pretty much good to go.

Anything else that needs to be changed?

samueleaton commented 6 years ago

Thanks for the reminder.