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

CI env variable displays warning if it's a string #1054

Open
altdsoy opened this issue Nov 17, 2024 · 2 comments
Open

CI env variable displays warning if it's a string #1054

altdsoy opened this issue Nov 17, 2024 · 2 comments

Comments

@altdsoy
Copy link

altdsoy commented Nov 17, 2024

Hello,

I'm not sure if the description is good enough, but I'm using a CI tool (Woodpecker CI) that injects the CI ENV variable with a string value.

It seems that when hex is checking its configuration it also checks this ENV.

In the following commit: 5eb05bd , its value is checked against a boolean:

def to_boolean(nil), do: {:ok, nil}
def to_boolean(false), do: {:ok, false}
def to_boolean(true), do: {:ok, true}
def to_boolean("0"), do: {:ok, false}
def to_boolean("1"), do: {:ok, true}
def to_boolean("false"), do: {:ok, false}
def to_boolean("true"), do: {:ok, true}
def to_boolean("FALSE"), do: {:ok, false}
def to_boolean("TRUE"), do: {:ok, true}
def to_boolean(_), do: :error

In my case because I don't have control on this ENV variable, Hex ends up considering it's an error and I'm getting an annoying warning on every mix command in my CI pipeline:

Invalid Hex config, falling back to default. Source: environment variable CI= "woodpecker".

I would like to open a PR but I wanted to discuss for an idiomatic way to solve this.

We can simply assume a truthy value for this ENV in particular (maybe using something like to_truthy_boolean) ?

@supersimple
Copy link
Member

That does sound annoying.
It is also an interesting behavior by woodpecker to not allow their predefined env vars to be modified.
Admittedly, this could just as easily my classified as a design error in their product as a deficiency in the Hex API.
If we were to make a change, my suggestion would be to write a special function like this to deal with the CI value (and treat it as you said, truthy or non-truthy. But, keeping that logic hidden inside of a private function.)
If you put up a PR, I am compelled to approve it. @ericmj @wojtekmach any objections?

@wojtekmach
Copy link
Member

Are you able to overwrite that value eg CI=true mix deps.get or set

environment:
  CI: true

Or something along those lines?

But if not, your patch is fine by me, I think it’d be nice on principle to continue being stricter but in this particular case I can’t see how it could be harmful.

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

No branches or pull requests

3 participants