search

JuliaRegistries / CompatHelper.jl

Automatically update the [compat] entries for your Julia package's dependencies
https://JuliaRegistries.github.io/CompatHelper.jl/dev/
MIT License
138 stars 39 forks source link

CompatHelper version 3: major rewrite and overhaul of the entire package (i.e. merge the `v3-dev` branch into the `master` branch) #361

Closed DilumAluthge closed 3 years ago

bors[bot] commented 3 years ago

try

Build failed:

bors[bot] commented 3 years ago

try

Build failed:

DilumAluthge commented 3 years ago

@fchorney @mattBrzezinski I think we should figure out these integration test failures before merging.

It looks like it's just that we're hitting some rate limits during the integration tests? If that's the case, we should do one or both of:

  1. Wrap the relevant tests in a bounded retry loop, maybe with an exponential backoff.
  2. Insert some sleep statements between tests
fchorney commented 3 years ago

weird that we haven't his these rate limits before during testing. Did GitHub change them? or is there some other reason we are hitting them?

fchorney commented 3 years ago

Hmm it also seems like it's the master 3 test every time. Maybe something is wonky with that test specifically

mattBrzezinski commented 3 years ago

Looking at the GitHub Documentation

additional rate limits may apply to some actions when using the API. For example, using the API to rapidly create content, poll aggressively instead of using webhooks, make multiple concurrent requests, or repeatedly request data that is computationally expensive may result in secondary rate limiting.

There is no getting around these limits. They do not interfere with the normal API. We need to retry / sleep here. Personally I think doing a sleep is a bit better as we're not spamming GitHub repeatedly. I'll make a PR for these changes.

mattBrzezinski commented 3 years ago

And to follow up Best Practices for Integrators says to wait one second between requests here. If you're still hitting it, do a retry using the Retry-After header from the response.

DilumAluthge commented 3 years ago

Anyway, once you've fixed the tests, this is good to go from my point of view.

I would say do the following steps:

  1. Fix whatever needs fixing.
  2. Run bors try and make sure it passes.
  3. After you have a successful bors try run, convert this PR from "draft" to "ready to review".
  4. Do bors merge.
  5. Once the PR has been merged, you can register a new release with JuliaRegistrator.
  6. Wait for the General PR to be merged.
  7. Make an announcement on Discourse.
codecov[bot] commented 3 years ago

Codecov Report

Merging #361 (887ddfb) into master (0afb9df) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #361   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           16        11    -5     
  Lines          409       337   -72     
=========================================
- Hits           409       337   -72
Impacted Files Coverage Δ
src/dependencies.jl 100.00% <100.00%> (ø)
src/exceptions.jl 100.00% <100.00%> (ø)
src/main.jl 100.00% <100.00%> (ø)
src/pull_requests.jl 100.00% <100.00%> (ø)
src/utilities/ci.jl 100.00% <100.00%> (ø)
src/utilities/git.jl 100.00% <100.00%> (ø)
src/utilities/new_versions.jl 100.00% <100.00%> (ø)
src/utilities/ssh.jl 100.00% <100.00%> (ø)
src/utilities/timestamps.jl 100.00% <100.00%> (ø)
src/utilities/types.jl 100.00% <100.00%> (ø)
... and 10 more

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 0afb9df...887ddfb. Read the comment docs.

mattBrzezinski commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build succeeded:

mattBrzezinski commented 3 years ago

bors r+

bors[bot] commented 3 years ago

Build succeeded: