OpenAstronomy / baldrick

I have a cunning plan!
https://baldrick.readthedocs.io
MIT License
14 stars 8 forks source link

ENH: Add base branch checker for new PR #92

Closed pllim closed 3 years ago

pllim commented 4 years ago

To address astropy/astropy#9385

codecov-io commented 4 years ago

Codecov Report

Merging #92 into master will increase coverage by 0.26%. The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
+ Coverage   82.34%   82.61%   +0.26%     
==========================================
  Files          17       18       +1     
  Lines         912      926      +14     
==========================================
+ Hits          751      765      +14     
  Misses        161      161
Impacted Files Coverage Δ
...ldrick/plugins/github_pull_requests_base_branch.py 71.42% <71.42%> (ø)
baldrick/plugins/github_pull_requests.py 87.91% <0%> (+4.39%) :arrow_up:

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 116546d...84a6feb. Read the comment docs.

bsipocz commented 4 years ago

@pllim - I don't know the API enough to review this, but the the gist of it as far as I see is indeed what I wanted to see as the feature :)

pllim commented 4 years ago

Sometimes it's legit to open a PR against non master.

Oh, then maybe we also need to check if some "skip base branch check" label (or something like that) exists, so we can skip this check? @bsipocz , do you have a preferred name for such a label?

bsipocz commented 4 years ago

I don't have preferences (and honestly, as the first draft, I'm happy to leave such PRs with the failing check, e.g. we would notice easily that something is not OK, but then can decide that it's actually deliberate). I would guess the rate is somewhere between 1-3PR/100PRs.

bsipocz commented 4 years ago

(But it'll be less now, that we don't have a separate bugfix and much dated LTS branch)

pllim commented 4 years ago

I don't understand why the tests are passing, LoL. I thought this one would fail because I forgot to update it after changing the behavior to only post status on failure:

    def test_good_base(self, app):
        self.pr_handler.base_branch = 'master'

        with app.app_context():
            sta = check_base_branch(self.pr_handler, self.repo_handler)

        sta['basebranch']['state'] == 'success'
pllim commented 4 years ago

anything else that needs doing here?

I don't remember... This was opened almost a year ago. 😬

codecov-commenter commented 4 years ago

Codecov Report

Merging #92 into master will increase coverage by 0.43%. The diff coverage is 88.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
+ Coverage   83.11%   83.54%   +0.43%     
==========================================
  Files          18       19       +1     
  Lines         983     1003      +20     
==========================================
+ Hits          817      838      +21     
+ Misses        166      165       -1     
Impacted Files Coverage Δ
...ldrick/plugins/github_pull_requests_base_branch.py 88.23% <88.23%> (ø)
baldrick/github/github_api.py 68.54% <0.00%> (-0.02%) :arrow_down:
baldrick/plugins/github_pull_requests.py 88.04% <0.00%> (+4.34%) :arrow_up:

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 5ab5202...1cbcee1. Read the comment docs.

pllim commented 4 years ago

Feel free to test it out before putting it in production.