-
Notifications
You must be signed in to change notification settings - Fork 48
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
Refactor text formatting functions #1418
base: master
Are you sure you want to change the base?
Refactor text formatting functions #1418
Conversation
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.
if you want bikeshedding... here's some extreme bikeshedding for you...!
I feel like trying to follow XKit 7's pattern of grouping utils just for the sake of grouping them was kinda silly, and I regret doing it. Rather than think up names for collections of utils which don't even share any code, I think the easier and clearer thing is to have one export per util file, e.g.
src/util/construct_relative_time_string.js
- Defines
thresholds
- Defines
relativeTimeFormat
- Exports
constructRelativeTimeString
- Defines
src/util/date_time_format.js
- Exports
dateTimeFormat
- Exports
src/util/elements_as_list.js
- Exports
elementsAsList
- Exports
Of course, this pattern doesn't work for util files which export several interdependent values (like user_blogs.js
), but at least that way we're only doing the group-naming work when it's actually necessary.
Spoken like a true developer-whose-project-at-work-uses-a-bundler, I kid, I kid, don't sue me, I'm innocent :P I certainly don't think I would go unequivocally to the opposite extreme, where one winds up with a folder full of function names in alphabetical order. If a set of functions will usually be used together or would need all need to be refactored at the same time, I think it makes sense to be able to find one from the other via context. But yeah, I think there's certainly places for it. |
Description
Should result in no functional changes. This extracts some utility functions that are used in the modals of various scripts into a utility file to reduce repetition.
Open to bikeshedding (different file name, interface.js, modals.js, whatever).
Testing steps