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

implement github change hook ci skipping #3443

Merged
merged 14 commits into from Aug 21, 2017

Conversation

Projects
None yet
4 participants
@iblis17
Contributor

iblis17 commented Jul 15, 2017

Contributor Checklist:

  • I have updated the unit tests
  • I have created a file in the master/buildbot/newsfragment directory (and read the README.txt in that directory)
  • I have updated the appropriate documentation
@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Jul 15, 2017

@iblis17, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rodrigc, @sa2ajj and @tardyp to be potential reviewers.

mention-bot commented Jul 15, 2017

@iblis17, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rodrigc, @sa2ajj and @tardyp to be potential reviewers.

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Jul 15, 2017

@iblis17, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rodrigc, @sa2ajj and @tardyp to be potential reviewers.

mention-bot commented Jul 15, 2017

@iblis17, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rodrigc, @sa2ajj and @tardyp to be potential reviewers.

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Jul 15, 2017

Codecov Report

Merging #3443 into master will increase coverage by <.01%.
The diff coverage is 95.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3443      +/-   ##
==========================================
+ Coverage   88.25%   88.26%   +<.01%     
==========================================
  Files         323      323              
  Lines       33719    33754      +35     
==========================================
+ Hits        29760    29794      +34     
- Misses       3959     3960       +1
Impacted Files Coverage Δ
master/buildbot/www/hooks/github.py 95.12% <95.12%> (+0.54%) ⬆️

Continue to review full report at Codecov.

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

codecov bot commented Jul 15, 2017

Codecov Report

Merging #3443 into master will increase coverage by <.01%.
The diff coverage is 95.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3443      +/-   ##
==========================================
+ Coverage   88.25%   88.26%   +<.01%     
==========================================
  Files         323      323              
  Lines       33719    33754      +35     
==========================================
+ Hits        29760    29794      +34     
- Misses       3959     3960       +1
Impacted Files Coverage Δ
master/buildbot/www/hooks/github.py 95.12% <95.12%> (+0.54%) ⬆️

Continue to review full report at Codecov.

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

@iblis17 iblis17 referenced this pull request Jul 15, 2017

Closed

FreeBSD CI #20585

@iblis17

This comment has been minimized.

Show comment
Hide comment
@iblis17

iblis17 Jul 16, 2017

Contributor

Seems GitHub will send two events (push and pull-request) at the same time when user add more commits to an opened PR.

Contributor

iblis17 commented Jul 16, 2017

Seems GitHub will send two events (push and pull-request) at the same time when user add more commits to an opened PR.

@iblis17 iblis17 changed the title from implement github change hook ci skipping to WIP: implement github change hook ci skipping Jul 16, 2017

@iblis17

This comment has been minimized.

Show comment
Hide comment
@iblis17

iblis17 Jul 16, 2017

Contributor

The payload of pull-request event does not contain the head commit message, how can I send a request to get the commit message in Twisted?

(here is a sample of payload: https://gist.github.com/iblis17/fc3d905b71f6aea2c2791e1a6010891e

Contributor

iblis17 commented Jul 16, 2017

The payload of pull-request event does not contain the head commit message, how can I send a request to get the commit message in Twisted?

(here is a sample of payload: https://gist.github.com/iblis17/fc3d905b71f6aea2c2791e1a6010891e

@tardyp

This comment has been minimized.

Show comment
Hide comment
@tardyp

tardyp Jul 16, 2017

Member

@iblis17 would it make sense to rather parse the PR comment?

Member

tardyp commented Jul 16, 2017

@iblis17 would it make sense to rather parse the PR comment?

@tardyp

This comment has been minimized.

Show comment
Hide comment
@tardyp

tardyp Jul 16, 2017

Member

note that there is this PR #3404 which fetches more info from a PR

Member

tardyp commented Jul 16, 2017

note that there is this PR #3404 which fetches more info from a PR

@iblis17

This comment has been minimized.

Show comment
Hide comment
@iblis17

iblis17 Jul 16, 2017

Contributor

would it make sense to rather parse the PR comment?

I want the [ci skip] key word behaves like Travis CI or Appveyor...

note that there is this PR #3404 which fetches more info from a PR

I think I can borrow some code from it! Thanks!

Contributor

iblis17 commented Jul 16, 2017

would it make sense to rather parse the PR comment?

I want the [ci skip] key word behaves like Travis CI or Appveyor...

note that there is this PR #3404 which fetches more info from a PR

I think I can borrow some code from it! Thanks!

@iblis17

This comment has been minimized.

Show comment
Hide comment
@iblis17

iblis17 Jul 18, 2017

Contributor

@tardyp finally, I got the pull_request skip checking work.
I will add tests cases later.

Contributor

iblis17 commented Jul 18, 2017

@tardyp finally, I got the pull_request skip checking work.
I will add tests cases later.

@tardyp tardyp closed this Jul 22, 2017

@tardyp tardyp reopened this Jul 22, 2017

@iblis17 iblis17 changed the title from WIP: implement github change hook ci skipping to implement github change hook ci skipping Jul 25, 2017

@iblis17

This comment has been minimized.

Show comment
Hide comment
@iblis17

iblis17 Jul 25, 2017

Contributor

Test cases added. Now this PR is ready for review.

Note that I changed fakeMasterForHooks (in ca3d902) in order to get httpclientservice work in TestCase.

Contributor

iblis17 commented Jul 25, 2017

Test cases added. Now this PR is ready for review.

Note that I changed fakeMasterForHooks (in ca3d902) in order to get httpclientservice work in TestCase.

@iblis17

This comment has been minimized.

Show comment
Hide comment
@iblis17

iblis17 Jul 25, 2017

Contributor

:/ how can I access that file?

Spelling checker messages written to /buildbot/buildbot-job/build/master/docs/_build/spelling/output.txt
build finished with problems.
Makefile:146: recipe for target 'spelling' failed
Contributor

iblis17 commented Jul 25, 2017

:/ how can I access that file?

Spelling checker messages written to /buildbot/buildbot-job/build/master/docs/_build/spelling/output.txt
build finished with problems.
Makefile:146: recipe for target 'spelling' failed
@tardyp

This comment has been minimized.

Show comment
Hide comment
@tardyp

tardyp Jul 25, 2017

Member

Looks (almost) good.

@s0undt3ch this will clash with your current PR. Can you please do additional review?

Member

tardyp commented Jul 25, 2017

Looks (almost) good.

@s0undt3ch this will clash with your current PR. Can you please do additional review?

Show outdated Hide outdated master/buildbot/newsfragments/gh-changehook-ci-skipping.feature
def fakeMasterForHooks():
master = Mock()
master = fakemaster.make_master()

This comment has been minimized.

@tardyp

tardyp Jul 25, 2017

Member

👍

@tardyp
Show outdated Hide outdated master/buildbot/test/unit/test_www_hooks_github.py
Show outdated Hide outdated master/buildbot/www/hooks/github.py
@iblis17

This comment has been minimized.

Show comment
Hide comment
@iblis17

iblis17 Jul 25, 2017

Contributor

This HTTPClientService got shared now?

Contributor

iblis17 commented on master/buildbot/www/hooks/github.py in e1a034c Jul 25, 2017

This HTTPClientService got shared now?

hook/github: configurable api_root_url
- including TestChangeHookConfiguredWithCustomApiRoot
- new options for GitHubEventHandler
    - ``github_api_endpoint``
    - ``debug``
    - ``verify``
Show outdated Hide outdated master/buildbot/www/hooks/github.py
Show outdated Hide outdated master/buildbot/www/hooks/github.py
Show outdated Hide outdated master/buildbot/www/hooks/github.py
Show outdated Hide outdated master/buildbot/www/hooks/github.py

@tardyp tardyp merged commit 43af7de into buildbot:master Aug 21, 2017

31 checks passed

bb Build done.
Details
bb/coverage/2.7/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/coverage/3.5/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/docs/2.7/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/flake8/2.7/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/flake8/3.5/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/isort/2.7/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/js/2.7/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/pylint/2.7/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/smokes/2.7/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/smokes/3.5/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/trial/2.7/tw:14.0.2/sqla:latest/db:sqlite:// Build done.
Details
bb/trial/2.7/tw:15.4.0/sqla:latest/db:sqlite:// Build done.
Details
bb/trial/2.7/tw:15.5.0/sqla:0.8.0/db:sqlite:// Build done.
Details
bb/trial/2.7/tw:15.5.0/sqla:latest/db:sqlite:// Build done.
Details
bb/trial/2.7/tw:latest/sqla:latest/db:mysql+mysqldb://travis@127.0.0.1/bbtest Build done.
Details
bb/trial/2.7/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/trial/2.7/tw:latest/sqla:latest/db:sqlite:////tmp/test_db.sqlite Build done.
Details
bb/trial/3.4/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/trial/3.6/tw:latest/sqla:latest/db:sqlite:// Build done.
Details
bb/trial_worker/2.6/tw:14.0.2/sqla:latest/db:sqlite:// Build done.
Details
bb/trial_worker/2.6/tw:15.4.0/sqla:latest/db:sqlite:// Build done.
Details
bb/trial_worker/2.7/tw:10.2.0/sqla:latest/db:sqlite:// Build done.
Details
bb/trial_worker/2.7/tw:11.1.0/sqla:latest/db:sqlite:// Build done.
Details
bb/trial_worker/2.7/tw:12.2.0/sqla:latest/db:sqlite:// Build done.
Details
bb/trial_worker/2.7/tw:13.2.0/sqla:latest/db:sqlite:// Build done.
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 95.12% of diff hit (target 88.25%)
Details
codecov/project 88.26% (+<.01%) compared to c7cffa1
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tardyp tardyp referenced this pull request Aug 21, 2017

Closed

GitHub PR's proper ownership and changed files #3404

0 of 3 tasks complete
@iblis17

This comment has been minimized.

Show comment
Hide comment
@iblis17

iblis17 Aug 22, 2017

Contributor

tnx a lot!

Contributor

iblis17 commented Aug 22, 2017

tnx a lot!

@iblis17 iblis17 deleted the iblis17:gh-hook-skip branch Nov 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment