-
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rename "display_settings" to "preferences" in web folder #30067
base: main
Are you sure you want to change the base?
Rename "display_settings" to "preferences" in web folder #30067
Conversation
Thanks for the fix! Could you please update your commit messages to match the commit style guidelines? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made a mistake, as I tried to change a commit message's description but accidentally commented here instead.
I am now editing this comment so it's not non-sensical.
I do not fully know how to edit commit messages through the github app, so I'll sift and find out how to do it through the terminal.
a471f17
to
6e6f65a
Compare
Making progress, I got this. |
I started by identifying the different types of "display_settings" and their references, all according to an earlier message in this issue (zulip#26874) from Tim Abbot. After analyzing the code for a solid few hours I determined that there were 3: a constant variable defined in server_events_dispatch.js; a few dictionaries in different files along with a filename; and a file named user_display_settings.hbs. Regarding this specific commit, I just had find all instances of the user_display_settings variable that wasn't the user_display_settings.hbs, and since all those instances were inside the web folder, I didn't have to worry about coupling issues, so all instances were simply changed to user_preferences.
5f444ca
to
920ea56
Compare
Finding all references to the 'display_settings' was a little hard due to my lack of extensive front-end knowledge, but I toughed it out through 3 different commits designed for the same purpose. This commit was previously 3 commits, so they were sqaushed for better readability and coherence. Once again these changes were all isolates to the web folder so no coupling issues to worry about.
This was by far the easiest commit, as I just changed a file name and it's singular reference in the settings tab list, which were both in the web folder so no coupling problems.
920ea56
to
aa573a4
Compare
Wow, very messy over here... But I think I solved all issues? |
Abandoning the issue related to this PR so I can get cracking on another issue I've had my eyes on for a while. |
Hopefully fixed the last conflict now |
What... is this conflict?? |
Heads up @Someone12543, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
Aptly renamed all instances of "display_settings" to "preferences" inside the web folder, as mentioned through an earlier message from timabbott that included a git grep command. Intended to solve all mentioned instances of "display_settings" in that same message.
Fixes part of: #26874
Self-review checklist
(variable names, code reuse, readability, etc.).
Communicate decisions, questions, and potential concerns.
Individual commits are ready for review (see commit discipline).
Completed manual review and testing of the following: