Distinguishes that there is an analysis error or there is not found app.conf #3
Conversation
| @@ -36,10 +38,14 @@ func LoadContext(confName string, confPaths []string) (*Context, error) { | |||
| var err error | |||
| var conf *Config | |||
| for _, confPath := range confPaths { | |||
| conf, err = ReadDefault(path.Join(confPath, confName)) | |||
| filePath := path.Join(confPath, confName) | |||
There was a problem hiding this comment.
Please use filepath.Join(confPath, confName).
|
@jeevatkm I guess |
| @@ -36,10 +38,14 @@ func LoadContext(confName string, confPaths []string) (*Context, error) { | |||
| var err error | |||
| var conf *Config | |||
| for _, confPath := range confPaths { | |||
| conf, err = ReadDefault(path.Join(confPath, confName)) | |||
| filePath := path.Join(confPath, confName) | |||
| conf, err = ReadDefault(filePath) | |||
| if err == nil { | |||
There was a problem hiding this comment.
Can you please change it to below:
if err != nil {
return nil, fmt.Errorf("%v: %v", filePath, err)
}
return &Context{config: conf}, nilWhile reviewing this PR, I noticed LoadContext method only loads a first confPath from confPaths. So It needs an attention. I will create an issue for that separately.
|
@alkchr - currently Regarding label |
|
@yuki2006 Thanks for contributing to Revel. Can you please take care of comments, |
@jeevatkm Yeah, my mistake. I didn't pay enough attention to the repo name.
From that perspective that makes sense. Though a new label can be introduced that would indicate that "LGTM" from core team is being awaited. |
|
Sorry. I used |
|
@yuki2006 no worries. I will be merging this PR. Also I will add line no of where that ini error occurred. For example: |
|
Manually targeted and merged to |
（Sorry for my poor English.)
"revel run project " is said to always be "no such file or directory" when you run even if there is a problem on app.conf.
example
https://raw.githubusercontent.com/yuki2006/revel_helloworld/master/conf/app.conf
It has been changed to the following