Closed RRickkert closed 6 years ago
Oh right I forgot to look at this on the computer. Yes this is what I was trying to say on discord with the properties with event in their name. I didn't really like it when i saw it.
@RRickkert I think the point is renaming a class IS a breaking change.
I don't think adding Event to any of the classes is needed as they are not events, it is the object used in the event. You look at almost every event in dotnet it using "Args" as suffix. However in this case I would say not needed? but that depends on others opinions
@LuckyNoS7evin I meant to say that the property still exists. That it's not removed. So that specifically isn't a breaking change. The renaming is indeed a breaking change, but then again, when would / wouldn't you allow such things? You'd basically be saying: We can't clean up code, since it might break someone's code, and we don't want the user to use a different class name. So we'll never change it.
If that's not what we'd want in general. I think it's a good idea to say, we'll never ever do such a thing. Because I've seen it happen before in changes, small renames, removing/adding a letter to a class.
Maybe we can set some rules for it then?
Working on removing the "Event" suffix by the way. 👍
Also @LuckyNoS7evin I've never seen the Args suffix be used professionally anywhere other than when used in a Event. As in EventHandler<[name]Args>. Maybe it could also be used as a class instance that you could pass to a constructor to a class, but in any other cases, like this, it makes even less sense than using the Event Suffix.
@LuckyNoS7evin @Mahsaap @swiftyspiffy Reverted the Events suffix. Could you guys re-check?
@swiftyspiffy I just reverted every single class name to what it was before this pull request. So whether it's a good thing to change some names or not, doesn't matter anymore. At least, not for this pull request.
Okay
I agree with swifty on naming the classes {X}Event. In theory they should be Args or EventArgs. However in a lot of cases the models are used outside the specific event. I know mine are at least.
I do agree that some renaming does need to take place however don't feel we need to add a suffix to the class names in this case.
Now the whisper one is interesting, the issue is with the rename of the object as well as making it obsolete it just breaks software rather than doing it nicely. We should exclude he class name change and this would be fine