-
Notifications
You must be signed in to change notification settings - Fork 91
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
chore(just): organize just recipes into modules and add comments to all #2002
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job - this makes things a great deal clearer.
Would be good to have newlines at the end of each file. I also generally prefer to not prefix usage examples with$
as it tends to make the commands harder to copy-paste.
Also, it would be nice to have a consistent approach to private recipes, either decorating with [private]
or (my preference) prefixing the name with a leading underscore.
None of these things are blocking though!
charts/README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One note that the just files contained in here were fairly purposeful, as this whole folder including readme (as well as the dev folder) gets published separately to https://github.com/astriaorg/charts
We used to have a seperate charts/dev-cluster repo and combining them was meant to make it so devs could test locally easier, but still have a repo where a third party could pull and run with dev files.
We would want to get the appropriate chart related justfiles published appropriately there as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added back to the charts
directory in 4a8278e
…cipe issue via more specific naming
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM now, albeit there are still a few oddities which we probably have to live with (comment formatting, duplication of clean
recipes as delete
recipes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved noting we will need a follow up PR to fix justfile in chart repo.
Summary
Leverages justfile's module system to better organize our recipes and adds descriptive and uniform comments to all.
Background
These changes are meant to make the developer local deployment experience easier to understand and use. One primary goal is for
just --list
to provide a more helpful list of recipes. This example shows the new style ofjust deploy
:The oddity of the extra line in the comment is to enforce that single line comments aren't placed to the right of the recipe, which causes formatting problems.
Another goal of these changes is to make the recipes themselves easier to navigate by splitting into submodules and following clear styling throughout (which aids greatly in identifying recipes if one doesn't have
just
syntax highlighting).The final aim of these changes is to make the command line outputs easier to understand by hiding a lot of extraneous information, such as outputs and just recipe calls (automatically listed usually).
Changes
just
directory to charts, containing modulesclean
,delete
,deploy
,evm
,init
, andrun
.just
directory to dev, containing modulesargo
,helm
, andkubetail
.[doc]
to support multiline comments.@
and summarized withecho
statements.wait-for-*
recipe calls intodeploy
recipes so that they are automatically called when a chart is deployed.Testing
Manually tested all recipes (
deploy::auctioneer
anddeploy::hyperlane-agents
aren't working, but locally they didn't work before, either. I've added "FIXME" to each)Changelogs
No updates required.
Related Issues
closes #1992