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

Add Makefile documentation #50

Merged
merged 7 commits into from
Aug 15, 2023
Merged

Add Makefile documentation #50

merged 7 commits into from
Aug 15, 2023

Conversation

ddelange
Copy link
Contributor

@ddelange ddelange commented May 17, 2023

Motivation

#38 (comment)

Modifications

Add help texts

Result

all                       - Alias for `build`
build                     - Build runtime docker image
develop                   - Build develop docker image and run an interactive shell in the develop envionment
fmt                       - Run formatting
help                      - Print Makefile documentation
run                       - Build develop docker image and run a make command in the develop envionment (e.g. `make run fmt` will execute `make fmt` within the docker container)
test                      - Run tests

@kserve-oss-bot kserve-oss-bot requested review from njhill and rafvasq May 17, 2023 06:39
@ddelange
Copy link
Contributor Author

cc @ckadner

ddelange added 3 commits May 17, 2023 08:44
Signed-off-by: ddelange <[email protected]>
Signed-off-by: ddelange <[email protected]>
@ckadner ckadner self-requested a review May 19, 2023 17:25
@ckadner ckadner self-assigned this May 19, 2023
@ckadner
Copy link
Member

ckadner commented May 19, 2023

cc @ckadner

Thanks @ddelange this is great. I will add a more detailed review soon.

@ddelange
Copy link
Contributor Author

friendly ping @ckadner :)

Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Thank you @ddelange -- appologies for the long wait.

I have a few minor wording suggestions and a regex improvement :-)

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@ckadner
Copy link
Member

ckadner commented Aug 2, 2023

What do you think about shortening the first column and adding some color?

help:
	@perl -0 -nle 'printf("\033[36m  %-15s\033[0m %s\n", "$$2", "$$1") while m/^##\s*([^\r\n]+)\n^([\w.-]+):[^=]/gm' $(MAKEFILE_LIST) | sort
image image

ddelange added a commit to ddelange/rest-proxy that referenced this pull request Aug 3, 2023
ddelange added a commit to ddelange/modelmesh-serving that referenced this pull request Aug 3, 2023
@ddelange
Copy link
Contributor Author

ddelange commented Aug 5, 2023

@ckadner done in all three PRs 👍

Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Thank you @ddelange

/lgtm

@kserve-oss-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ckadner, ddelange
To complete the pull request process, please assign tjohnson31415 after the PR has been reviewed.
You can assign the PR to them by writing /assign @tjohnson31415 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ckadner ckadner merged commit 1aee152 into kserve:main Aug 15, 2023
@ckadner ckadner added this to the v0.11.1 milestone Oct 2, 2023
@ckadner ckadner assigned ddelange and unassigned ckadner Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants