Use .localeapprc as config filename

[xijo]

.env is a general purpose name and might be taken by other app
specific environment settings. Let's use a name that actually says what
this file is used for.

[michaelbaudino]

Hi @xijo and thanks for your contribution.

Would you mind describing why .env is a problem for you? Before going deeper into the implementation details, I'd like to be sure what problem we are trying to solve here.

Feel free to describe your whole usecase and/or workflow and the details of the issue you are trying to solve: I'd be happy to understand it and help.

[xijo]

Hi @michaelbaudino

in one of our repos the .env directory is considered to contain environment specific variables, like secrets and the like, which are then exported via direnv.

.env sounds like a general purpose name for a file so I'd say its pretty likely to be taken in other contexts as well. Additionally why not name the file according to what it does or contains, so in case of localeapp this would be something like .localeapprc or .localeapp_key or something.

I went with the the former because it fits into the UNIX pattern of dot rc files containing startup information for the according application.

[xijo]

@michaelbaudino What's the status of this PR?

[michaelbaudino]

Hi @xijo,

First, sorry for the response delay 😞

We've discussed your suggestion with the team and we have got a few remarks and questions for you, if you don't mind:

1. We totally agree that reading .env should not be our gem responsibility, we introduced it a while back and now regret it, to be honest.
2. Any change we make should be backward-compatible as much as possible (so your change should actually not change the elsif clause, but add a new one, even if it clearly lacks of elegancy).
3. Can you confirm that direnv reads the .env directory? (we can only see mentions of a .envrc file in their documentation but we're not familiar with this tool so we are probably missing something)
4. Since you are loading .env using direnv, why isn't LOCALEAPP_API_KEY available from your environment (the code you are modifying here should stop at the first elsif clause and not even reach your modification, shouldn't it?)
5. Writing the API key in the .env file is something we did to improve/ease the setup instructions (it avoids the "Add your API key to your .env file" step), but we don't do it by default anymore (there's a localeapp install --write-env-file switch, now).
6. In order to better understand what you are trying to achieve, can you tell me if you ultimately want to:
   1. set LOCALEAPP_API_KEY differently for the CLI and for your application?
   2. set LOCALEAPP_API_KEY via a fourth mechanism (not via the -k|--api-key switch, neither via the environment directly, neither from .env)
   3. unset LOCALEAPP_API_KEY even if it is present in a config file (in which case, maybe a --no-read-env switch could be an option: it's backward-compatible and doesn't had another elsif clause, what do you think?)
   4. something else?

I know it's a lot of questions, and I'm sorry, but we're really trying to understand your needs here, without breaking backward compatibility.

[xijo]

Hi @michaelbaudino,

Thanks for your response 👍 I'll try to clarify my concerns a bit more.

First of all: I do not have any problem nor am I trying to change the way LOCALEAPP_API_KEY gets set. My localeapp config lives under .localeapp/config.rb.

I do have several projects that contain a .env folder with application specific logic. The usage of direnv is only a (more or less) clarifying example.

Within these projects it is currently not possible to use the localeapp cli since it treats the folder as a file and crashes when attempting to read it.
We could fix this by checking the file a bit more carefully. This sounds like a good way to go for a backward compatible fix.

Still, a configuration file should be properly named. I'd go for .localeapprc or deprecate the whole feature because it is quite similar to what .localeapp/config.rb does.

The names of run-control files, their higher-level syntax, and the semantic interpretation of the syntax are usually application-specific.

-- The Art of Unix Programming
http://www.faqs.org/docs/artu/ch10s03.html#ftn.id2941902

Thanks and best,
Jo

[michaelbaudino]

Gotcha! Your ultimate purpose is good practice hygiene: we love that 😍

And OK, I totally get that the CLI crashes when .env is a directory: we should have been more rigorous about that and use File.file? rather than File.exist? 😕 We'll get that fixed quickly 👍 (so at least you shouldn't have to maintain your own fork/branch anymore)

In terms of good practices, the right approach would finally be a breaking change (i.e. a major version bump) that reads neither .env nor any other file: it would just try to retrieve LOCALEAPP_API_KEY from the environment, and it would be up to the user to provide it via the environment the way they like (direnv, .env read by foreman or the dotenv gem, passed to Docker, …). What do you think?

[xijo]

Sounds good 👍

I wouldn't necessarily say that having a config file is bad. Since you'll keep the .localeapp/config.rb anyway (right?) I'd just fallback to read the token from there.
Which is what you already do. So maybe just deprecate and ditch the whole .env reading and everything's fine!

[michaelbaudino]

@xijo Version 3.1.2 should fix the .env issue (crash when it's a directory) 👍 (#272)

I'll close this for now, but I've created an issue to stop reading .env completely in a future major (i.e. with breaking changes) release 🎩

[xijo]

Great, thanks 👍