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

--config-toml argument #296

Merged
merged 4 commits into from
Jan 15, 2025
Merged

--config-toml argument #296

merged 4 commits into from
Jan 15, 2025

Conversation

Canop
Copy link
Owner

@Canop Canop commented Jan 11, 2025

Fix #284

Feedback welcome

@c-git
Copy link
Contributor

c-git commented Jan 11, 2025

Going to test now, I really love how nicely you integrated the idea into the code. It feels obvious what to expect and I think that's desirable. Regarding your comment on the downside

A downside of the chosen solution is that if you specify a job, and it's not the default job, it's not the one executed, which may be surprising for who isn't used to the multi job system of bacon.

I guess only time will tell if that actually materializes as a problem but my thinking is that this is not likely to be a problem. Most of the time people will be running one liner commands they found in tutorials or documentation not writing it by hand. And it's pretty easy to identify the problem if they provide the command.

@c-git
Copy link
Contributor

c-git commented Jan 11, 2025

Once we finalize we'll also need to include an example on the website. Just mentioning it before I forget.

It works as expected but there are a few gotchas that I found while trying it out.

  • You need to wrap the TOML in single quotes otherwise you need to escape all the double quotes.
  • TOML needs white space so you must use a multi line command.

Those are just statements to capture what I observed but I still think this provides the most flexibility and adds the ability to share commands that people can copy and paste. It allows you to iterate on your job in the bacon.toml then copy and paste it into a command line command. Overall I'm happy with it.

This is my test command that did indeed work as desired

bacon -j cli-test --config-toml '
[jobs.cli-test]
command = [
  "sh",
  "-c",
  "echo \"hello $(date +%H-%M-%S)\"; cargo run",
]
need_stdout = true
allow_warnings = true
background = false
on_change_strategy = "kill_then_restart"
kill = ["pkill", "-TERM", "-P"]'

Thank you very much.

@Canop
Copy link
Owner Author

Canop commented Jan 15, 2025

I think I'll just add your example in the cookbook page.

Hard to call this a "one liner", though.

@c-git
Copy link
Contributor

c-git commented Jan 15, 2025

That's fair but it' is something you can copy and paste and just try without multiple steps.

@Canop
Copy link
Owner Author

Canop commented Jan 15, 2025

That's fair but it' is something you can copy and paste and just try without multiple steps.

Yes. I was just wondering how to call that thing... "bacon snippet" ?

@c-git
Copy link
Contributor

c-git commented Jan 15, 2025

Yeah maybe "bacon inline command job snippet" ? But that does seem long.

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.

Pass the command for a job as an argument to bacon
2 participants