Skip to content
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

Bump hex_core to v0.10.1 #335

Merged
merged 3 commits into from
Jun 12, 2024
Merged

Conversation

starbelly
Copy link
Member

@starbelly starbelly commented Jun 11, 2024

Updates to the latest hex_core. Oddly enough, a spec in hex_core that has never caused problems before caused problems today. The spec is here : https://github.com/hexpm/hex_core/blob/8a53ac8eddaf425cd70a41ae9279318063c01e67/src/hex_api_release.erl#L19

Not sure why now, but I'll open an issue in relevant places to sort it out.

The spec in hex_core for hex_api_release:retire/4 indicates
atoms are the keys for the retire reason (a map). Yet, we've
always used strings and they continue to work.
@starbelly starbelly marked this pull request as ready for review June 11, 2024 23:04
@starbelly starbelly requested a review from ferd June 11, 2024 23:53
@paulo-ferraz-oliveira
Copy link
Collaborator

paulo-ferraz-oliveira commented Jun 12, 2024

a spec in hex_core that has never caused problems

The last CI I see for this is 8 months old. 24.3.4.17 is 2 months old. Might it be Erlang/OTP that changed something that caused that issue?

Edit: maybe it'd also be a nice time to include 26 and 27 in CI? (if you prefer, though, I can do a separate pr for that); and potentially, while we're at it, actions/checkout to v4, as well as codecov/codecov-action to v4, and also rebar3 to 3.23.0; yeah, I'll do a separate pull request for this. --> #336


{profiles, [
{test, [
{extra_src_dirs, ["test/support"]},
{overrides, [{override, rebar3,[{deps, [{erlware_commons, "1.3.1"}]}]}]},
{deps, [{hex_core, "0.8.4"},
{deps, [{hex_core, "0.10.1"},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd get rid of this one. Doesn't rebar3 just accumulate deps in profiles (i.e. concatenate them with whatever's in the "default" profile)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this was put in play to test a different version from what rebar3 had, but in rebar3 it is now vendored.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this should be possible to take out.

@paulo-ferraz-oliveira
Copy link
Collaborator

paulo-ferraz-oliveira commented Jun 12, 2024

@starbelly, I'm also gonna remove -dialyzer directives from this repo. and fix the issues as I go along (edit: #338). Some of them might come from hex_core (at least one spec. is wrong) and I'll post a version in a pull request that uses another hex_core I'll pull request to fix 😄 --> hexpm/hex_core#148

Copy link
Collaborator

@paulo-ferraz-oliveira paulo-ferraz-oliveira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you'll want to remove hex_core from the test profile, but let that not affect merging+moving on. 👍

@paulo-ferraz-oliveira
Copy link
Collaborator

Once this one's merged I'll go ahead and rebase the others.

@starbelly
Copy link
Member Author

Maybe you'll want to remove hex_core from the test profile, but let that not affect merging+moving on. 👍

Yes, let's get this merged and open up a tidy PR as a follow on, we also should modify CI to test on OTP 26/27 and stop testing 24.

@starbelly starbelly merged commit a21c817 into erlef:main Jun 12, 2024
2 checks passed
@paulo-ferraz-oliveira
Copy link
Collaborator

Good stuff. I already have a couple PRs lined up. I didn't remove '25' from tests, since "current-2" seems to be acceptable for OTP versions under test. At the same time, though, in one of the PRs, I did remove hex_core from the test profile, as cleanup. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants