fix(events): app run event wording #362
Conversation
EtienneM
force-pushed
the
fix/326/app_run_event_wording
branch
2 times, most recently
from
9734756
to
b12bddb
Compare
EtienneM
force-pushed
the
fix/326/app_run_event_wording
branch
from
b12bddb
to
45bf7c5
Compare
events_structs.go
Outdated
| if ev.User.Email == "deploy@scalingo.com" { | ||
| return "Scalingo Operator" | ||
| } |
There was a problem hiding this comment.
@Soulou I updated the Who method to match what is asked in the related issue. But not that it also impacts other event types. For instance in case of a crash or an alert. Is it OK for you?
There was a problem hiding this comment.
Could we keep something more generic like Scalingo Platform? (It seems to me it's already what is in the event no?)
It's weird to say an operator crashes a container
There was a problem hiding this comment.
The current output is <scalingo-platform (deploy@scalingo.com)>, with this change it would be <Scalingo Operator>. I wanted to follow what is asked on the related issu but I agree that this feels a bit odd. Or maybe I could try to modify it only for the run event?
There was a problem hiding this comment.
For the run event I agree, for these rest it's misleading..
There was a problem hiding this comment.
OK that's what is included in the commit 4782dab
events_structs.go
Outdated
| return fmt.Sprintf("%s (%s)", ev.User.Username, ev.User.Email) | ||
| } | ||
|
|
||
| func (ev *Event) PrintableType() string { | ||
| return strings.Title(strings.Replace(string(ev.Type), "_", " ", -1)) | ||
| return cases.Title(language.English).String(strings.Replace(string(ev.Type), "_", " ", -1)) |
There was a problem hiding this comment.
strings.Title is deprecated, hence this change.
EtienneM
force-pushed
the
fix/326/app_run_event_wording
branch
6 times, most recently
from
de589f1
to
30461a8
Compare
EtienneM
force-pushed
the
fix/326/app_run_event_wording
branch
from
30461a8
to
4272bae
Compare
| return fmt.Sprintf("%sone-off container with command '%s'", detached, ev.TypeData.Command) | ||
| } | ||
|
|
||
| func (ev *EventRunType) isEventRunFromOperator() bool { |
There was a problem hiding this comment.
Isn't it more if the comande is run by the generic user?
There was a problem hiding this comment.
What do you mean by "generic user"?
|
With commit 4782dab, only the run event has a custom user name. So the output is: We can see |
EtienneM
requested review from
Soulou and
curzolapierre
and removed request for
yohann-bacha
|
@curzolapierre I let you assigned as reviewer. I wanted Soulou's thoughts about the wording but I want your review on the code part :) |
events_structs_test.go
Outdated
| @@ -48,6 +48,22 @@ var eventsSpecializeCases = map[string]struct { | |||
| DetailedEventName: "*scalingo.EventEditAppType", | |||
| DetailedEventString: "application settings have been updated, Force HTTPS has been disabled", | |||
| }, | |||
| "test app run event for a command run by an opeator": { | |||
There was a problem hiding this comment.
| "test app run event for a command run by an opeator": { | |
| "test app run event for a command run by an operator": { |
EtienneM
force-pushed
the
fix/326/app_run_event_wording
branch
from
4782dab
to
0c9788b
Compare
|
Sorry @curzolapierre, pushing your suggestion dismissed your review.. |
I also took the opportunity to fix all linter offenses.
Fix #326