feat(cli): discover additional app-config yamls #2484
Conversation
|
| 'app-config.janus-idp.yaml', | ||
| 'app-config.backstage-community.yaml', | ||
| 'app-config.example.yaml', |
There was a problem hiding this comment.
I'm not sure about looking for app-config.example.yaml and even the existing app-config.yaml entry that was originally here. One thing I'd like to avoid is accidentally picking up one of these files that may contain a complete backstage configuration file for this.
There was a problem hiding this comment.
I can remove app-config.yaml, but please be aware that some plugins uses this currently.
But since this feature is not documented (or?), I'm do what you prefer.
Should I remove it? :)
There was a problem hiding this comment.
Yep, we haven't documented this convention at all yet, so we can decide what we want to keep in the longer term. I'm not sure of the origin of even the app-config.janus-idp.yaml convention, either the intent was it's a file that is meant to be handled by the janus-idp CLI or maybe the intent was just to match it with the organization name, like what was done when plugins were moved to the community repo and this app-config.backstage-community.yaml convention was introduced.
That being said, the more I think about it, the more I think maybe it's better to just keep it simple and maybe we should eventually just narrow down this list to app-config.yaml despite my worry that an entire yaml config gets picked up by the tooling, it's much easier to remember :-)
There was a problem hiding this comment.
or better still as @davidfestal suggested app-config.dynamic.yaml as the one we document and favor over the other fallbacks, I think that naming makes it pretty clear it's not like the other config files.
There was a problem hiding this comment.
Maybe this log statement could be updated as part of this too to mention that.
Signed-off-by: Christoph Jerolimov <[email protected]>
christoph-jerolimov
force-pushed
the
discover-additional-app-configs
branch
from
31e2ce0 to
1c06b84
Compare
|
|
This probably needs a changeset |
| return val; | ||
| }, undefined); | ||
| .map(fileName => path.join(directory, fileName)) | ||
| .find(candidate => fs.existsSync(candidate)); |
Prefers:
Wdyt? :)