introduce `make help` as default make recipe #3888

STRRL · 2023-01-09T13:15:23Z

What problem does this PR solve?

close #3769

What's changed and how it works?

In this PR, we provide a target called make help like this:

Related changes

This change also requires further updates to the website (e.g. docs)
This change also requires further updates to the UI interface
Need to cheery-pick to release branches
- release-2.5
- release-2.4

Checklist

CHANGELOG

I have updated the CHANGELOG.md
I have labeled this PR with "no-need-update-changelog"

Tests

Unit test
E2E test
No code
Manual test (add steps below)

Side effects

Breaking backward compatibility

DCO

If you find the DCO check fails, please run commands like below (Depends on the actual situations. For example, if the failed commit isn't the most recent) to fix it:

git commit --amend --signoff
git push --force

ti-chi-bot · 2023-01-09T13:15:26Z

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

codecov · 2023-01-09T13:32:10Z

Codecov Report

Merging #3888 (36ae2f0) into master (e5c03c2) will decrease coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3888      +/-   ##
==========================================
- Coverage   38.62%   38.53%   -0.09%     
==========================================
  Files         167      167              
  Lines       13734    13734              
==========================================
- Hits         5305     5293      -12     
- Misses       7998     8003       +5     
- Partials      431      438       +7

see 7 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7c633a...36ae2f0. Read the comment docs.

Signed-off-by: STRRL <im@strrl.dev>

STRRL · 2023-04-12T06:53:19Z

/cc @g1eny0ung

PTAL

g1eny0ung · 2023-04-12T07:00:13Z

I will look at it this afternoon and this evening.

g1eny0ung

The rest LGTM. As it's too late, I'll check it again this morning.

Makefile

cmd/generate-makefile/generate_container_image_makefile.go

cmd/generate-makefile/doc.go

g1eny0ung · 2023-04-12T17:05:09Z

hack/local-up-chaos-mesh.sh

@@ -107,7 +107,7 @@ fi

 if [ -z "$SKIP_IMAGE_BUILD" ]; then
    echo "info: building docker images"
-    IMAGE_REGISTRY_PREFIX=$IMAGE_REGISTRY_PREFIX IMAGE_PROJECT=chaos-mesh IMAGE_TAG=$IMAGE_TAG UI=1 SWAGGER=1 make image
+    IMAGE_REGISTRY_PREFIX=$IMAGE_REGISTRY_PREFIX IMAGE_TAG=$IMAGE_TAG UI=1 SWAGGER=1 make image


Suggested change

IMAGE_REGISTRY_PREFIX=$IMAGE_REGISTRY_PREFIX IMAGE_TAG=$IMAGE_TAG UI=1 SWAGGER=1 make image

IMAGE_TAG=$IMAGE_TAG UI=1 SWAGGER=1 make image

If I understand correctly, now we are no longer providing the ability to customize the registry, and some targets with docker-push prefix also are being removed.

I agree with these changes, since it's easy to rename the image name with docker image tag, we don't need to keep these seemingly convenient, but somewhat redundant targets.

So the IMAGE_REGISTRY_PREFIX env should be removed here as well as others.

If I understand correctly, now we are no longer providing the ability to customize the registry, and some targets with docker-push prefix also are being removed.

Yeah, that's the thing I want to do: do NOT provide the flags/ENVs for configuring different registry/repo, but only during the building process.

It seems the changes in local-up-chaos-mesh.sh is NOT a part of building things but installing things.

So maybe it's better to revert the changes in local-up-chaos-mesh.sh.

If necessary, I would open another PR for similar changes, but for installing scripts like install.sh, local-up-chaos-mesh.sh and other .sh scripts.

What do you think about it? Reverting changes in local-up-chaos-mesh.sh

What do you think about it? Reverting changes in local-up-chaos-mesh.sh

I still don't think it's necessary because these scripts are used more for debugging purposes, and giving them more configurable items is often redundant. As I said above:

I agree with these changes, since it's easy to rename the image name with docker image tag, we don't need to keep these seemingly convenient, but somewhat redundant targets.

So, keep modifying it like this. :) 🔧

@STRRL Only this place is not updated, after the update I will approve this PR.

Hi @g1eny0ung , I actually prefer to revert changes in local-up-chaos-mesh.sh in this PR.

I totally agree with your opinion. But it's a little out of the scope from this PR.

I would do it, but in another new PR. Not carry changes in .sh files in the PR.

What do you think about it?

But it's a little out of the scope from this PR.

@STRRL That sounds reasonable. I agree that we can wrap up the .sh modifications in the upcoming PR.

cmd/generate-makefile/generate_container_image_makefile.go

cmd/generate-makefile/funcmaps.go

…tion Signed-off-by: STRRL <im@strrl.dev>

Signed-off-by: STRRL <im@strrl.dev>

…enerating Signed-off-by: STRRL <im@strrl.dev>

cwen0 · 2023-04-18T13:53:31Z

Cool! I have run this pr on the environment, The make help is very useful!

Signed-off-by: STRRL <im@strrl.dev>

g1eny0ung

LGTM!

g1eny0ung · 2023-04-23T02:51:49Z

/merge

g1eny0ung · 2023-04-23T03:17:22Z

@chaos-mesh/maintainers @chaos-mesh/committers Please be aware that any additions or removals of make targets in the Makefile must now be done with consideration for modifying the template.

ti-chi-bot added the do-not-merge/work-in-progress label Jan 9, 2023

ti-chi-bot requested a review from Hexilee January 9, 2023 13:15

ti-chi-bot added the size/XL label Jan 9, 2023

ti-chi-bot added size/XXL and removed size/XL labels Jan 11, 2023

STRRL changed the title ~~use make help which print help message as default make recipe~~ introduce make help which print help message as default make recipe Jan 11, 2023

ti-chi-bot added the needs-rebase label Mar 13, 2023

STRRL force-pushed the make-help branch 3 times, most recently from 6eefdae to 25fc391 Compare April 3, 2023 12:33

STRRL changed the title ~~introduce make help which print help message as default make recipe~~ introduce make help as default make recipe Apr 3, 2023

ti-chi-bot added needs-rebase and removed needs-rebase labels Apr 4, 2023

STRRL added 12 commits April 12, 2023 14:03

feat: generation binary.generated.mk

c32ed4c

Signed-off-by: STRRL <im@strrl.dev>

feat: adapt to origin slash in the target name

e0d637e

Signed-off-by: STRRL <im@strrl.dev>

chore: refine makefile

12c2e7d

Signed-off-by: STRRL <im@strrl.dev>

feat(makefile): complete make help command

70acc6c

Signed-off-by: STRRL <im@strrl.dev>

build: fix gnu make parallel build

f18b38a

Signed-off-by: STRRL <im@strrl.dev>

ci: fix license checker

7a43abe

Signed-off-by: STRRL <im@strrl.dev>

chore: remove unused chaos-jvm

42d95ab

Signed-off-by: STRRL <im@strrl.dev>

build: simplify make image recipe

0130c2d

Signed-off-by: STRRL <im@strrl.dev>

build: make help with category

7d5b93e

Signed-off-by: STRRL <im@strrl.dev>

chore: cleanup env end with _REGISTRY and _PROJECT

b116e7d

Signed-off-by: STRRL <im@strrl.dev>

chore: update Changelog

1961863

Signed-off-by: STRRL <im@strrl.dev>

chore: cleanup env end with _REGISTRY and _PROJECT

0d5a1e3

Signed-off-by: STRRL <im@strrl.dev>

STRRL force-pushed the make-help branch from 25fc391 to 0d5a1e3 Compare April 12, 2023 06:10

ti-chi-bot removed the needs-rebase label Apr 12, 2023

STRRL marked this pull request as ready for review April 12, 2023 06:37

ti-chi-bot removed the do-not-merge/work-in-progress label Apr 12, 2023

ti-chi-bot requested a review from g1eny0ung April 12, 2023 06:53

g1eny0ung added the component/scripts label Apr 12, 2023

g1eny0ung reviewed Apr 12, 2023

View reviewed changes

g1eny0ung reviewed Apr 13, 2023

View reviewed changes

cmd/generate-makefile/funcmaps.go Show resolved Hide resolved

g1eny0ung requested review from cwen0 and removed request for Hexilee April 13, 2023 06:35

STRRL added 3 commits April 13, 2023 21:49

chore(makefile): use longer spaces to seperate the target and descrip…

31474d8

…tion Signed-off-by: STRRL <im@strrl.dev>

chore: fix typos

73eda62

Signed-off-by: STRRL <im@strrl.dev>

chore: empty string between target and dependencies during makefile g…

6118355

…enerating Signed-off-by: STRRL <im@strrl.dev>

revert: changes in local-up-chaos-mesh.sh

3283519

Signed-off-by: STRRL <im@strrl.dev>

g1eny0ung mentioned this pull request Apr 22, 2023

feat!: build binaries locally with local/ prefix targets in Makefile #4004

Merged

11 tasks

g1eny0ung approved these changes Apr 23, 2023

View reviewed changes

g1eny0ung added the require-LGT1 label Apr 23, 2023

Merge branch 'master' into make-help

36ae2f0

g1eny0ung merged commit 0b31a51 into chaos-mesh:master Apr 23, 2023

STRRL mentioned this pull request Apr 25, 2023

cheat sheet of useful commands while developing #1963

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

introduce `make help` as default make recipe #3888

introduce `make help` as default make recipe #3888

STRRL commented Jan 9, 2023 •

edited

Loading

ti-chi-bot commented Jan 9, 2023

codecov bot commented Jan 9, 2023 •

edited

Loading

STRRL commented Apr 12, 2023

g1eny0ung commented Apr 12, 2023

g1eny0ung left a comment

g1eny0ung Apr 12, 2023

STRRL Apr 13, 2023

g1eny0ung Apr 13, 2023

g1eny0ung Apr 13, 2023 •

edited

Loading

g1eny0ung Apr 17, 2023

STRRL Apr 21, 2023

g1eny0ung Apr 21, 2023

cwen0 commented Apr 18, 2023

g1eny0ung left a comment

g1eny0ung commented Apr 23, 2023

g1eny0ung commented Apr 23, 2023

	IMAGE_REGISTRY_PREFIX=$IMAGE_REGISTRY_PREFIX IMAGE_TAG=$IMAGE_TAG UI=1 SWAGGER=1 make image
	IMAGE_TAG=$IMAGE_TAG UI=1 SWAGGER=1 make image

introduce make help as default make recipe #3888

introduce make help as default make recipe #3888

Conversation

STRRL commented Jan 9, 2023 • edited Loading

What problem does this PR solve?

What's changed and how it works?

Related changes

Checklist

DCO

ti-chi-bot commented Jan 9, 2023

codecov bot commented Jan 9, 2023 • edited Loading

Codecov Report

STRRL commented Apr 12, 2023

g1eny0ung commented Apr 12, 2023

g1eny0ung left a comment

Choose a reason for hiding this comment

g1eny0ung Apr 12, 2023

Choose a reason for hiding this comment

STRRL Apr 13, 2023

Choose a reason for hiding this comment

g1eny0ung Apr 13, 2023

Choose a reason for hiding this comment

g1eny0ung Apr 13, 2023 • edited Loading

Choose a reason for hiding this comment

g1eny0ung Apr 17, 2023

Choose a reason for hiding this comment

STRRL Apr 21, 2023

Choose a reason for hiding this comment

g1eny0ung Apr 21, 2023

Choose a reason for hiding this comment

cwen0 commented Apr 18, 2023

g1eny0ung left a comment

Choose a reason for hiding this comment

g1eny0ung commented Apr 23, 2023

g1eny0ung commented Apr 23, 2023

introduce `make help` as default make recipe #3888

introduce `make help` as default make recipe #3888

STRRL commented Jan 9, 2023 •

edited

Loading

codecov bot commented Jan 9, 2023 •

edited

Loading

g1eny0ung Apr 13, 2023 •

edited

Loading