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

Only run coverage on merges/pushes to master #1783

Merged
merged 1 commit into from
May 24, 2017

Conversation

ethantkoenig
Copy link
Member

@ethantkoenig ethantkoenig commented May 23, 2017

Our CI builds are really slow (routinely over 40 minutes), and running the unit tests takes a large portion of the CI duration.

From running locally, it seems that running tests without coverage reports is much faster (~6x) than running with coverage reports (partly because they can be run in parallel, partly because shared code only needs to be compiled once). We don't really need coverage reports for commits/changes that aren't part of master, so only run a coverage report when changes are pushed/merged to master.

Changes the make test to only runs test (no coverage report), and adds the make test-coverage rule for generating a coverage report.

NOTE: needs signature for changes to drone.yml

@sapk
Copy link
Member

sapk commented May 23, 2017

LGTM. This will need a new sign ( cc @lunny )

@tboerger tboerger added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label May 23, 2017
@lunny
Copy link
Member

lunny commented May 24, 2017

LGTM, @sapk it seems no need anymore?

@tboerger tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 24, 2017
@lunny lunny added this to the 1.2.0 milestone May 24, 2017
@ethantkoenig
Copy link
Member Author

@lunny The CI build (https://drone.gitea.io/go-gitea/gitea/3191) has a warning

@ethantkoenig ethantkoenig mentioned this pull request May 24, 2017
@appleboy
Copy link
Member

Please rebase master branch.

@appleboy
Copy link
Member

trust LGTM

@appleboy appleboy merged commit ff96bda into go-gitea:master May 24, 2017
@ethantkoenig ethantkoenig deleted the ci_build branch May 24, 2017 22:48
@tboerger
Copy link
Member

Bad idea to do it like that, plugins don't support commands attribute

@lunny
Copy link
Member

lunny commented May 28, 2017

@tboerger did you mean this PR will result the tag will not be released automatically?

@tboerger
Copy link
Member

The added commands should be ignored

@lunny lunny added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Aug 25, 2017
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants