elli-lib / elli

Simple, robust and performant Erlang web server
https://github.com/elli-lib/elli/blob/develop/doc/README.md
MIT License
314 stars 36 forks source link

Move support to OTP 22+ #114

Closed paulo-ferraz-oliveira closed 3 weeks ago

paulo-ferraz-oliveira commented 11 months ago

Closes #103.

This pull request is being split into several other pull requests, to ease review. These are listed below.

codecov-commenter commented 11 months ago

Codecov Report

Patch coverage: 82.35% and project coverage change: +0.07% :tada:

Comparison is base (3ec3522) 76.32% compared to head (829beda) 76.40%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #114 +/- ## ========================================== + Coverage 76.32% 76.40% +0.07% ========================================== Files 12 12 Lines 756 750 -6 ========================================== - Hits 577 573 -4 + Misses 179 177 -2 ``` | [Files Changed](https://app.codecov.io/gh/elli-lib/elli/pull/114?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/elli\_middleware.erl](https://app.codecov.io/gh/elli-lib/elli/pull/114?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2VsbGlfbWlkZGxld2FyZS5lcmw=) | `100.00% <ø> (ø)` | | | [src/elli\_sendfile.erl](https://app.codecov.io/gh/elli-lib/elli/pull/114?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2VsbGlfc2VuZGZpbGUuZXJs) | `62.50% <ø> (ø)` | | | [src/elli\_tcp.erl](https://app.codecov.io/gh/elli-lib/elli/pull/114?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2VsbGlfdGNwLmVybA==) | `86.66% <ø> (ø)` | | | [src/elli\_util.erl](https://app.codecov.io/gh/elli-lib/elli/pull/114?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2VsbGlfdXRpbC5lcmw=) | `100.00% <ø> (ø)` | | | [src/elli\_request.erl](https://app.codecov.io/gh/elli-lib/elli/pull/114?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2VsbGlfcmVxdWVzdC5lcmw=) | `78.49% <50.00%> (ø)` | | | [src/elli\_http.erl](https://app.codecov.io/gh/elli-lib/elli/pull/114?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2VsbGlfaHR0cC5lcmw=) | `68.13% <80.00%> (+0.02%)` | :arrow_up: | | [src/elli.erl](https://app.codecov.io/gh/elli-lib/elli/pull/114?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2VsbGkuZXJs) | `77.61% <100.00%> (ø)` | | | [src/elli\_example\_callback.erl](https://app.codecov.io/gh/elli-lib/elli/pull/114?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2VsbGlfZXhhbXBsZV9jYWxsYmFjay5lcmw=) | `90.62% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

paulo-ferraz-oliveira commented 11 months ago

~CI failing for OTP 26, 24, and 22. I'll try to replicate locally.~

paulo-ferraz-oliveira commented 11 months ago

~It's possible the failure for non-OTP 26 is due to #99, since re-executing the tests (in a fork) showed ✅.~

paulo-ferraz-oliveira commented 11 months ago

~Removed OTP 26 from scope, since there're SSL -related issues to solve. Should be moved to a new PR, I guess...~

tsloughter commented 11 months ago

Woo, these are all looking good.

And yea, for 26 the ssl certs may need to be re-created. I had to do this for chatterbox tests too.

paulo-ferraz-oliveira commented 11 months ago

Is anything lacking for this one to get merged? I can then rebase and update the other ones.

I think I'll deal with OTP 26 in a separate PR so as to not inflict further confusion in these pull requests... 😄

paulo-ferraz-oliveira commented 9 months ago

@tsloughter, if you prefer I can also reduce the scope for this and do multiple pull requests to an isolated branch until you're fully satisfied and then we merge to the main branch.

tsloughter commented 9 months ago

This doesn't look too bad, I just hadn't taken a close look.

paulo-ferraz-oliveira commented 3 weeks ago

@tsloughter, what's required to push this forward?

tsloughter commented 3 weeks ago

Hey sorry, it is just a large PR. But it looks like a lot of it is formatting?

paulo-ferraz-oliveira commented 3 weeks ago

👋 it's probably the commit related to Elvis, but I can try to break it down further.

paulo-ferraz-oliveira commented 3 weeks ago

I'm moving to draft, to be able to break it down into several pull requests. Once that's done, I'll close this one without it being merged.

paulo-ferraz-oliveira commented 3 weeks ago

All new pull requests mentioned in the top description.