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

optimize: optimize makefile targets and speed up docker build #3246

Conversation

TrafalgarZZZ
Copy link
Member

Ⅰ. Describe what this PR does

Optimize makefile targets and speed up docker build. The PR removes redundant generate fmt vet gen-openapi actions away from the original Makefile targets and do once before targets like make build, make docker-push-all.

Targets for building one single component will no longer check these actions generate fmt vet gen-openapi.

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
@cheyang
Copy link
Collaborator

cheyang commented May 22, 2023

/test fluid-e2e

@cheyang cheyang requested a review from zwwhdls May 22, 2023 10:03
@codecov
Copy link

codecov bot commented May 22, 2023

Codecov Report

Merging #3246 (510109c) into master (0f7d87c) will not change coverage.
The diff coverage is n/a.

❗ Current head 510109c differs from pull request most recent head a7b4339. Consider uploading reports for the commit a7b4339 to get more accurate results

@@           Coverage Diff           @@
##           master    #3246   +/-   ##
=======================================
  Coverage   65.53%   65.53%           
=======================================
  Files         397      397           
  Lines       23112    23112           
=======================================
  Hits        15147    15147           
  Misses       6183     6183           
  Partials     1782     1782           

@TrafalgarZZZ
Copy link
Member Author

/retest

2 similar comments
@TrafalgarZZZ
Copy link
Member Author

/retest

@TrafalgarZZZ
Copy link
Member Author

/retest

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
@TrafalgarZZZ
Copy link
Member Author

/retest

1 similar comment
@TrafalgarZZZ
Copy link
Member Author

/retest

@TrafalgarZZZ
Copy link
Member Author

/test fluid-e2e

1 similar comment
@TrafalgarZZZ
Copy link
Member Author

/test fluid-e2e

@zwwhdls
Copy link
Member

zwwhdls commented May 23, 2023

It's better to set gen-openapi check in github action just like "CRD validation check"

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
@TrafalgarZZZ
Copy link
Member Author

It's better to set gen-openapi check in github action just like "CRD validation check"

Moved from build CI task to lint CI task. Thx

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
8.4% 8.4% Duplication

Copy link
Collaborator

@cheyang cheyang left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@fluid-e2e-bot
Copy link

fluid-e2e-bot bot commented May 24, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheyang

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

The pull request process is described 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

@cheyang cheyang removed the request for review from zwwhdls May 24, 2023 02:12
@cheyang cheyang merged commit 37e17c6 into fluid-cloudnative:master May 24, 2023
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.

3 participants