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

fix loading .env by composer #91

Merged
merged 1 commit into from
May 27, 2022
Merged

Conversation

aleksey-novikov
Copy link

This fix early load .env file to override some variables like FILENAME_TEMPLATE

@No9
Copy link
Collaborator

No9 commented May 27, 2022

Thanks for this - Good catch
Rather than loading the .env file twice we should probably just change the order of the call in

let mut cc = config::CoreConfig::new()?;

So

    let mut cc = config::CoreConfig::new()?;
    cc.set_namespace("default".to_string());
    let mut envloadmsg = String::from("Loading .env");
    let l_dot_env_path = cc.dot_env_path.clone();
    match dotenv::from_path(l_dot_env_path) {
        Ok(v) => v,
        Err(e) => envloadmsg = format!("no .env file found so using Debug level logging {}", e),
    }

Should be

    let mut envloadmsg = String::from("Loading .env");
    let l_dot_env_path = cc.dot_env_path.clone();
    match dotenv::from_path(l_dot_env_path) {
        Ok(v) => v,
        Err(e) => envloadmsg = format!("no .env file found so using Debug level logging {}", e),
    }
    let mut cc = config::CoreConfig::new()?;
    cc.set_namespace("default".to_string());

Let me know what you think

@No9
Copy link
Collaborator

No9 commented May 27, 2022

@aleksey-novikov I'm going to bundle up some changes this weekend
Do you want to apply the suggested change or are you ok for me to land it an close this?

@No9 No9 merged commit e972a26 into IBM:main May 27, 2022
@No9
Copy link
Collaborator

No9 commented May 27, 2022

OK I am assuming you've rightly checked out for the weekend - I am going to merge this so the contribution is recognized then apply refactor afterwards.

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.

2 participants