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

Configure watch via .amber.yml #996

Merged
merged 3 commits into from
Dec 22, 2018

Conversation

anamba
Copy link
Contributor

@anamba anamba commented Dec 19, 2018

Description of the Change

Allow user to configure amber watch from .amber.yml. Configuring this way is optional (there is still a default config, and it is the same as before), and you can continue to use the command line switches if you want (if you have a config in .amber.yml, command line switches will override it).

A sample .amber.yml:

type: app
language: ecr
model: granite

# list of tasks to be run by `amber watch`
watch:
  # NOTE: names that match crystal commands are special (e.g. run, spec)
  run:
    # commands will be joined with && (join them yourself if need || or ;)
    build_commands:
      - mkdir -p bin
      - crystal build ./src/rcwebcr.cr -o bin/rcwebcr
    run_commands:
      - bin/rcwebcr
    include:
      - ./config/**/*.cr
      - ./src/**/*.cr
      - ./src/**/*.ecr
    exclude: # NOTE simplistic implementation: (1) enumerate all includes and excludes; (2) return (includes - excludes)
      - ./src/dont_restart_when_i_am_changed.cr
  spec:
    run_commands:
      - crystal spec
    include:
      - ./spec/**/*.cr
    exclude:
      - ./spec/dont_run_specs_when_i_am_changed.cr
  npm:
    build_commands:
      - npm install --loglevel=error
    run_commands:
      - npm run watch

Alternate Designs

The config is too complex to configure solely via command line switches/arguments, and leaving it non-user-configurable is creating unhappiness among users.

Benefits

Developers are now free to choose yarn over npm, or even nothing at all. Additionally, they can use amber watch to run other processes as well.

Possible Drawbacks

Mainly some code bloat. It's certainly possible that this is not something the amber project should handle directly.

…mplate for details); rewrote ProcessRunner to handle any generic task that can be specified by a set of build and run commands and a set of globs to include and exclude; removed unused build_args and run_args args
@robacarp
Copy link
Member

@anamba thanks for doing this, it's long overdue and will be a welcome change to the workflow.

It's certainly possible that this is not something the amber project should handle directly.

This is a great thing to discuss. Can you explain what you mean by that? Are there existing tools which Amber can wrap or depend on that you have experience with? I think previously Amber depended on another shard, which I believe was abandoned/absorbed.

Copy link
Member

@drujensen drujensen left a comment

Choose a reason for hiding this comment

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

Excellent work. 🥇

@anamba
Copy link
Contributor Author

anamba commented Dec 19, 2018

This is a great thing to discuss. Can you explain what you mean by that? Are there existing tools which Amber can wrap or depend on that you have experience with? I think previously Amber depended on another shard, which I believe was abandoned/absorbed.

Nah, I didn't have anything in mind, I just mentioned that for the sake of completeness (answering the question).

I think you're referring to Sentry (which is still around). I saw some references to Sentry in there as I was working on this.

I do use Guardian for specs sometimes (but now that won't be necessary for amber projects). The main advantage provided by amber watch (with my changes) vs guardian is that it restarts the server process when it dies or fails to start properly (which for me, is surprisingly often, several times per work session... not sure if that's something to look into, or just one more weird Docker thing).

@drujensen drujensen merged commit 05afdce into amberframework:master Dec 22, 2018
@anamba anamba deleted the watch-improvements branch December 22, 2018 03:57
@faustinoaq
Copy link
Contributor

@anamba Thanks you so much for this! 💯

I think we should document this here: https://docs.amberframework.org/amber/cli/watch

Related issue: amberframework/docs#121

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.

4 participants