Skip to content

fix: exposed generic functionality to be re-used#174

Merged
rakeshJn merged 1 commit intolakevision-project:mainfrom
juancappi:fix/make-refactor
Nov 14, 2025
Merged

fix: exposed generic functionality to be re-used#174
rakeshJn merged 1 commit intolakevision-project:mainfrom
juancappi:fix/make-refactor

Conversation

@juancappi
Copy link
Collaborator

No description provided.

Signed-off-by: Juan Cappi <juancappi@gmail.com>
@juancappi juancappi requested a review from jurossiar November 13, 2025 19:11
Copy link
Collaborator

@jurossiar jurossiar left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@jurossiar jurossiar left a comment

Choose a reason for hiding this comment

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

Could you also expose this to be re-used too?
VENV_DIR=be/.venv
VENV_PYTHON=$(VENV_DIR)/bin/python

@juancappi
Copy link
Collaborator Author

Could you also expose this to be re-used too? VENV_DIR=be/.venv VENV_PYTHON=$(VENV_DIR)/bin/python

Yes, but probably not worth the extra complexity. Spent some time trying to do the same we did for check-env but turns out they work quite differently and the same pattern can't be applied for those variables. check-venv works because it’s invoked at run time; we only need it when a target executes. VENV_DIR/VENV_PYTHON are different, because Make needs those values during parse time to expand other targets and variables. For what I found, there is no way to “call” a target in another Makefile and get a variable back before parsing finishes. There is a way to make it work, but involves moving those two assignments into a an .mk file, which would be shared by both Makefiles

@rakeshJn rakeshJn merged commit ce36ae7 into lakevision-project:main Nov 14, 2025
1 check passed
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.

3 participants