-
Notifications
You must be signed in to change notification settings - Fork 28
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
DM-48364: mobu - update docs #4054
Conversation
9dea6ea
to
d92bb91
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we talked about this, but I don't remember the result: I think we decided that we were going to inject this value via Argo CD based on the environment configuration so that, eventually, no one needs to set this manually, right?
dd05f8e
to
ad879e6
Compare
Yep, ideally no one has to set this manually, but I didn't think we decided exactly how we were going to do it. Is it possible to do it from the ArgoCD environment now? Other than doing something like parsing the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's besides this review but I noticed that Mobu has two GitHub Apps now. Did you consider having one app and putting either a configuration file in the target repos or configuration in Phalanx saying what operations are enabled on that repo?
5d98c5a
to
ebe07c8
Compare
review feedback Change raw URLs to markdown links here -> the Mobu documentation
ebe07c8
to
47ad162
Compare
A few reasons, not all of them particularly convincing :)
|
I think this is a placeholder pending service discovery? |
Yep, and the Argo CD injection was done in this PR. |
No description provided.