Fixes #21060 - re-factor installer messages to methods #540
Conversation
|
Issues: #21060 |
|
|
||
| def server_success_message(kafo) | ||
| success_message | ||
| say " * <%= color('Katello', :info) %> is running at <%= color('#{kafo.param('foreman', 'foreman_url').value}', :info) %>" |
There was a problem hiding this comment.
Would it make sense to avoid the use of Katello here? Either the application or the interface could work.
There was a problem hiding this comment.
You mean saying Foreman or Application ?
There was a problem hiding this comment.
If you avoid any branded words and use generic descriptions like Application then there's no difference between upstream & downstream which makes life easier.
There was a problem hiding this comment.
It does, but there are some instances where that is unavoidable such as the commands or package names. I can make as much as possible generic but it won't prevent this situation.
There was a problem hiding this comment.
That would be preferable. With commands I'd prefer a git-style application where you can brand the main binary (foreman-maintain -> satellite-maintain) and subcommands are unbranded. You are correct that in some cases it will be unavoidable but let's aim at minimizing that.
There was a problem hiding this comment.
Alright, I'll change as much as I can although I will lastly state that I think moving to generics while making this easier, reduces the user experience.
There was a problem hiding this comment.
The more I look at it, this is about the only line we save anything on, and thus its cleaner to just change this to 'Foreman' and leave the rest as is at the risk of just being overly generic and ambigous.
|
This little bug is related -- theforeman/kafo#223 |
|
👍 I tested this along with the kafo and downstream patches and it looks and works well. As for the making the messages more generic to avoid re-writing them when inherited I lean towards leaving the messages as specific and informative as possible. The reason is to keep the message clean and simple for the user. Also when re-used in different scenario it is likely that more information will be added/changed and it seems more sustainable to have the message less fragmented and replace it as a whole |
|
@ekohl What say you? |
|
Seems this is only failing with the known issue with librarian now. |
|
Looking at theforeman/kafo#223 and https://github.com/theforeman/foreman-installer/blob/develop/hooks/post/10-post_install_message.rb we might need to look at the bigger picture when we want to merge. On kafo master I'd expect you to get 2 post install messages now: one from the vanilla foreman and one from katello. Another thing I'm not sure I like is the Kafo::Helpers namespace. That's something in katello-installer but not foreman-installer. Should we provide helpers in kafo to display messages? It could prepare for i18n. |
|
I think there a number of things we've invented in kinstaller hooks that could/should be baseline Kafo functionality that one can just use. We've not ever looked at porting it back but should. We are still safe with that kafo change -- as https://github.com/Katello/katello-installer/blob/master/config/katello.yaml#L19 does not include the foreman-installer hooks. |
That's a fair point - I was going to solve http://projects.theforeman.org/issues/19986 in theforeman/foreman-installer#250 but I think I'll have to duplicate it in katello-installer then. |
|
@ekohl yep, how we've done it so far, once we merge installers we can solve that all once and for all. For now, status quo. Is this good to merge then? |
No description provided.