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

Simplify configs package #1032

Merged
merged 10 commits into from
Jun 7, 2019
Merged

Simplify configs package #1032

merged 10 commits into from
Jun 7, 2019

Conversation

NicolasMahe
Copy link
Member

Closes #1021

This PR simplify the config package by removing config that were used by the cli, flatten the Core struct, and make the Engine work with default value!

config/config.go Outdated Show resolved Hide resolved
@@ -33,13 +33,13 @@ func initDependencies() (*dependencies, error) {
}

// init services db.
serviceDB, err := database.NewServiceDB(filepath.Join(config.Core.Path, config.Core.Database.ServiceRelativePath))
serviceDB, err := database.NewServiceDB(filepath.Join(config.Path, config.Database.ServiceRelativePath))
Copy link
Contributor

Choose a reason for hiding this comment

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

config.Database.ServiceRelativePath should already contain config.Path

Copy link
Member Author

Choose a reason for hiding this comment

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

At the end it makes sense to keep config.Path out of config.Database.XXXXRelativePath so by only modifiying config.Path, it will change all other path that use this folder.
The user doesn't need to know all sub-folder.

Copy link
Contributor

@krhubert krhubert Jun 7, 2019

Choose a reason for hiding this comment

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

What I want to avoid is the fact that other package needs to modify configs path. In that case, only config package manages paths etc... What you wrote is true also when config package joins paths.

To be clear I don't want a user to specify the full path for the database. I want to move a join call into config package.

I know we have this in the previous version of engine, but it could be refactored and clear a bit if :)

@NicolasMahe
Copy link
Member Author

I moved config.Path to ~/.mesg so it's compatible within docker but also without docker! Really important in order to run test properly without having any hack.
The mesg folder should be mount inside docker in /home/root/.mesg. I will update JS CLI.

@NicolasMahe NicolasMahe requested a review from krhubert June 7, 2019 05:46
@antho1404 antho1404 merged commit b3e8dbb into dev Jun 7, 2019
@antho1404 antho1404 deleted the feature/configs branch June 7, 2019 09:02
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.

Fix dev scripts
3 participants