erlang-lager / lager

A logging framework for Erlang/OTP
Apache License 2.0
1.12k stars 456 forks source link

Further adapt to OTP 24 + adopt GHA #539

Closed paulo-ferraz-oliveira closed 3 years ago

paulo-ferraz-oliveira commented 3 years ago

This pull request builds on top of my previous one.

On the one hand, I prefer erl_anno:line/1 to matching on an "opaque" structure, since this eases maintenance.

On the other hand, with a more recent version of OTP (from master), I got new warnings (introduced by new behaviour from https://github.com/erlang/otp/pull/2995), now fixed.

Note: this doesn't fundamentally change the way we handle the location, but it meant I was assuming that the format for that might not change, which might not be true (since OTP 24 isn't out yet).

paulo-ferraz-oliveira commented 3 years ago

I'm pushing a ~hack~ "fix" to the tests, since I keep getting unexpected results (discussed here) that mislead me into thinking I added/changed something that start causing an issue.

paulo-ferraz-oliveira commented 3 years ago

Moving to draft, since I'm going to try and replace Travis CI with GitHub Actions (running here, for testing purposes).

paulo-ferraz-oliveira commented 3 years ago

I've removed gen_fsm from the scope of OTP 22+ tests.

paulo-ferraz-oliveira commented 3 years ago

After many a experiment with Github Actions and failing tests, I found that I was missing container option --user 1001. All tests are now passing, as evidenced by Embrace the future at https://github.com/paulo-ferraz-oliveira/lager/actions.

michaelklishin commented 3 years ago

@paulo-ferraz-oliveira beat me to some of these changes. This seems a bit more urgent now that OTP 24 has reached a public RC.

@mrallen1 merging this would help team RabbitMQ test on OTP 24 (and indirectly test Lager on it).

paulo-ferraz-oliveira commented 3 years ago

@michaelklishin: cool. The main goal was to prepare for OTP 24, but I thought I could fix the tests too, while moving to GitHub Actions, so I did.

We'd appreciate a release too, since MANY of our lib.s still depend on lager, though I'd really like to have https://github.com/erlang-lager/lager/pull/540 included in that release too. 😄

Vagabond commented 3 years ago

I don't know much about GHA, but as long as it's more reliable than Travis I don't object.

The changes look fine. I think we can probably just remove the platform_define entirely at this point.

@mrallen1 any thoughts?

paulo-ferraz-oliveira commented 3 years ago

@mrallen1 already told me (before) we don't need to keep supporting stuff prior to OTP 21, which is why the GHA element doesn't make a reference to it.

I'll remove it , and also propose further cleanup (I can see references to OTP 18, for example).

There you go: f2ae8fe.

paulo-ferraz-oliveira commented 3 years ago

@Vagabond, should we get rid of appveyor also?

jadeallenx commented 3 years ago

@paulo-ferraz-oliveira @lukebakken built the appveyor stuff to test lager on Windows Erlang builds (which I guess was something that RabbitMQ needed) but if it's bit rotted and broken, remove it

jadeallenx commented 3 years ago

@Vagabond @paulo-ferraz-oliveira Kill anything prior to OTP 20 with extreme prejudice - it's on the TODO list, just haven't TODONE it...

paulo-ferraz-oliveira commented 3 years ago

@mrallen1: appveyor is failing, but I'm not sure it's broken, which is why I asked 😄. I don't know what versions it runs on, or what's the target (the yml is not very informative).

Regarding OTP 20, I cleaned pre OTP-21 stuff from the tests, but didn't go after anything else.

I'm OK for merge if you are.

lukebakken commented 3 years ago

If I have time I'll re-visit a Windows build but yeah feel free to nuke it for now.

michaelklishin commented 3 years ago

@michaelklishin: cool. The main goal was to prepare for OTP 24, but I thought I could fix the tests too, while moving to GitHub Actions, so I did.

We'd appreciate a release too, since MANY of our lib.s still depend on lager, though I'd really like to have #540 included in that release too. 😄

No objections from me. I just hope we will have an OTP 24-compatible release in the next couple of weeks so that all Lager users can begin testing on the OTP 24 RC1, even if they build every dependency from source :)

jadeallenx commented 3 years ago

Thank you very much. I know this was a lot of work - really appreciate it.