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

refactor: reorder hack scripts #1750

Merged
merged 1 commit into from
Jul 20, 2018
Merged

refactor: reorder hack scripts #1750

merged 1 commit into from
Jul 20, 2018

Conversation

fuweid
Copy link
Contributor

@fuweid fuweid commented Jul 18, 2018

Ⅰ. Describe what this PR did

Reorder hack scripts and make the code coverage correct.

Ⅱ. Does this pull request fix one issue?

fixes #1582

Ⅲ. Describe how you did it

  • use hack/install dir to contains all about the dependencies install thing
  • use hack/testing dir to contains all about the integration thing.
  • use gometalinter to do all the linters

However, I still maintain the hack/build and hack/make.sh since both the dep and rpm packages need them. In the following the PR, we should remove the legacy code and refactor the package-scripts.

Besides this, I also refactor travis.yml to splits the testing into three jobs.

  • Unit-Test
  • Integration-Test
  • CRI-Integration-Test

Based on this, we can use the tag for the job coverage so that Codecov can combine three kinds of coverages into one. Please check the comment created by Codecov, like:

Flag Coverage Δ
#critest 33.52% <ø> (?)
#integrationtest 37.67% <ø> (?)
#unittest 20.33% <ø> (?)

Ⅳ. Describe how to verify it

Eye ball?

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Jul 19, 2018

Codecov Report

Merging #1750 into master will increase coverage by 41.98%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1750       +/-   ##
===========================================
+ Coverage   14.16%   56.14%   +41.98%     
===========================================
  Files         281      200       -81     
  Lines       56797    15657    -41140     
===========================================
+ Hits         8043     8790      +747     
+ Misses      47817     5769    -42048     
- Partials      937     1098      +161
Flag Coverage Δ
#critest 33.53% <ø> (?)
#integrationtest 37.27% <ø> (?)
#unittest 20.33% <ø> (?)
Impacted Files Coverage Δ
daemon/containerio/hijack_conn.go 77.14% <0%> (-11.43%) ⬇️
pkg/meta/store.go 54.76% <0%> (-4.77%) ⬇️
daemon/mgr/system.go 73.27% <0%> (-1.73%) ⬇️
daemon/logger/jsonfile/utils.go 71.54% <0%> (-1.63%) ⬇️
daemon/mgr/container_utils.go 56.27% <0%> (-0.44%) ⬇️
cli/ps.go
client/container_exec.go
client/container_upgrade.go
client/container_stop.go
client/network_connect.go
... and 125 more

@fuweid fuweid changed the title --wip-- [skip ci] refactor: reorder hack scripts Jul 19, 2018
* refactor hack/build, hack/make.sh and Makefile
* refactor .travis.yml to split testing into three jobs
* use gometalinter to do all the linters

Signed-off-by: Wei Fu <fhfuwei@163.com>
@sunyuan3
Copy link
Contributor

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jul 20, 2018
@sunyuan3 sunyuan3 merged commit 56dd0f5 into AliyunContainerService:master Jul 20, 2018
@fuweid
Copy link
Contributor Author

fuweid commented Jul 20, 2018

The change is very large. @YaoZengzeng , @Letty5411 and @chuanchang please take a look again. Thanks


cd "$(dirname "${BASH_SOURCE[0]}")"
source utils.sh

Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to use absolute dir instead of cd to source the utils.sh

@Letty5411
Copy link
Contributor

Another question is do we need to modify related docs in pouch/doc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
areas/test kind/refactor LGTM one maintainer or community participant agrees to merge the pull reuqest. size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[help wanted] reorder hack folder to make script easy to maintain
5 participants