ngraham added inline comments. INLINE COMMENTS
> tuxxi wrote in kmenuedit.cpp:30 > IMHO it looks cleaner to separate includes into categories, which is what I > did here. It surely is, but that's not related to this patch. In KDE, we try to make our commits as atomic as possible, and do clean-up like this separately. > tuxxi wrote in kmenuedit.cpp:141 > I copied the tooltip text from KOpenWithDialog, but I'll change this one. > Perhaps we can change it as well Yep, sounds like we should change it it there too! A good second patch. :) > tuxxi wrote in kmenuedit.cpp:143 > I'm using `auto` to avoid duplicating the type name when using `new` since > the type is right there; its a habit I got into since clang-tidy recommends > it. I couldn't find anything in the style guides prohibiting this. If you > _really_ want, I can stop using `auto`. If there's nothing specifically in the style guidelines about it, it's best to follow the existing coding style. Nothing else here uses `auto` with `new` constructors, so we should follow the same style for new code. KDE software is multi-generational and it's important that each individual developer not use their own personal preferred style instead of following the existing style because that leads to the whole codebase becoming an inconsistent mess over time. Cleanup can be desirable, but that should happen separately, in its own patch, so it can be discussed on its own merits. REPOSITORY R103 KMenu Editor REVISION DETAIL https://phabricator.kde.org/D18939 To: tuxxi, ngraham, #plasma, cfeck Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
