-
Notifications
You must be signed in to change notification settings - Fork 202
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
Add support for aliases to ov
#4409
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.
This is excellent, and works great locally! I also needed to run ov build && ov init
first, but after that ov welcome
ran fine. Interesting and cool workaround!
@@ -57,7 +57,7 @@ fi | |||
case "$_cmd" in | |||
init) | |||
"$_self" build | |||
"$_self" just install-hooks | |||
"$_self" 'just install-hooks && just init-ov-aliases' |
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.
I think the help text up above will also need to reference this change!
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.
Oops, yes it will, thanks!
Ah, of course. Hopefully the entrypoint doesn't need to change that often, but if we find it does for some reason, we can change it to a stub script that just passes through to a mounted script that doesn't need to be copied into the container. Something like # entrypoint.sh
"$OPENVERSE_PROJECT"/docker/dev_env/configure_environment.sh "$@"
python "$OPENVERSE_PROJECT"/docker/dev_env/exec.py "$@" Maybe? |
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.
LGTM. I would have preferred a more standard way to define aliases than using a Python script, but this works well so it's okay with me.
|
||
SHARED_ALIASES = { | ||
"j": ["just"], | ||
"nuxt": ["j", "p", "frontend", "dev"], |
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 feels like preferential treatment for the frontend 😄.
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.
It was just the first useful one I could think of that clearly illustrated multi-term expansion 😅
This is pretty cool! I also had to run However, I cannot commit after this without skipping pre-commit validations 😅
|
@krysal you have to link the sudo ln -s <path to repo>/ov /usr/local/bin/ov |
Oh, that's no good! The hooks shouldn't rely on |
Fixes
Fixes #4407 by @sarayourfriend
cc @AetherUnbound and @dhruvkb who are probably the most interested with this.
Description
An alternative to #4408 using a Python-based approach.
This approach has all the features of the bash approach in #4408, but none of the disadvantages. It shares the same overall problems like the aliases only being available in the entrypoint. They aren't available inside
just
recipes themselves, for example. I think that's fine though!Testing Instructions
Try
ov j
,ov nuxt
, andov welcome
. See if it works and makes sense!Checklist
Update index.md
).main
) or a parent feature branch.just catalog/generate-docs
for catalogPRs) or the media properties generator (
just catalog/generate-docs media-props
for the catalog or
just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin