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

[TOREE-553] Correct the behavior of make dev #221

Merged
merged 2 commits into from
Sep 2, 2024

Conversation

pan3793
Copy link
Member

@pan3793 pan3793 commented Sep 8, 2023

This PR aims to improve the developer experience of the command make dev.

Previously, after changing the code, running make dev does not pick up the new assembly artifacts, which is not convenient for developers.

After the change, make dev becomes intuitive, it always picks up the latest assembly artifacts.

@@ -161,7 +161,7 @@ dist: dist/toree pip-release
dev: DOCKER_WORKDIR=/srv/toree/etc/examples/notebooks
dev: SUSPEND=n
dev: DEBUG_PORT=5005
dev: .toree-dev-image dist
dev: dist .toree-dev-image
Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's a bug, we should do dist first, so that the Dockerfile's ADD command could see the artifacts.

ADD dist/toree /usr/local/share/jupyter/kernels/toree

Makefile Outdated Show resolved Hide resolved
@pan3793 pan3793 changed the title [TOREE-553] Improve experience of make dev [TOREE-553] Correct the behavior of make dev Sep 8, 2023
@pan3793
Copy link
Member Author

pan3793 commented Sep 8, 2023

@lresende, I found this issue during debug phase, would you please take a look?

@pan3793
Copy link
Member Author

pan3793 commented Aug 26, 2024

@lresende I have reverted the debatable change, can we consider getting the rest change in?

@lresende lresende merged commit f44ec78 into apache:master Sep 2, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants