search

lxqt / lxqt-notificationd

The LXQt notification daemon
https://lxqt.github.io
GNU Lesser General Public License v2.1
62 stars 38 forks source link

Fix naming #347

Closed stefonarch closed 9 months ago

stefonarch commented 1 year ago

Renamed Advanced Settings → General Settings and Basic Settings → Appearance Settings. Fixes https://github.com/lxqt/lxqt-notificationd/issues/346 Related https://github.com/lxqt/lxqt-notificationd/issues/289

tsujan commented 1 year ago

This kind of change can easily cause regressions, while its review isn't easy. Considering the fact that there is no issue in the app, I don't see why we should do this.

stefonarch commented 1 year ago

I'm aware of this, but working with the code as it is is quite confusing. I tested loading other translations, it looks working fine all.

tsujan commented 1 year ago

... is quite confusing

You'll easily get used to it after a while. It's much simpler than the main code.

Moreover, the GUI's order might be changed again later, and we don't want to make heavy changes each time.

stefonarch commented 1 year ago

Moreover, the GUI's order might be changed again later, and we don't want to make heavy changes each time.

I just think that the code filenames should respect what there are about. I don't think there will be much to reorder in future.

shlyakpavel commented 1 month ago

This kind of change can easily cause regressions, while its review isn't easy.

Apart from translations, what seems to be the problem? What kind of regressions do occur when changing class and file names? The only thing that comes to my mind are signals and slots, but they are referenced without obsolete macros, so that should be fine as well

tsujan commented 1 month ago

What kind of regressions do occur when changing class and file names?

Regressions could easily happen because we're fallible human beings.