Skip to content

Conversation

@JHWelch
Copy link
Contributor

@JHWelch JHWelch commented Oct 16, 2025

This PR updates instances of "Subclassed" and "Subclassing" with the more common "Override" and "Overriding"

It looks like these are the only places in the codebase that use this terminology. Though "subclass" as a verb is not incorrect, "Override" is much more common, even in this codebase it is used 60+ times.

This continues to make this codebase more accessible to future devs!

I'm not sure if there is any reason I should hold off on the Log.warn statements from the weatherprovider.js. Let me know and I can revert 3c540d3 if need.

@sdetweil
Copy link
Collaborator

sdetweil commented Oct 16, 2025

I don’t think Override is any better.
while technically correct , the base function is a do nothing

so I think
Provide a function named
Is a better practical set of words

most of our module developers are not programmers

@rejas
Copy link
Collaborator

rejas commented Oct 16, 2025

I am fairly indifferent to these changes to be honest, its just wording and both are fine IMHO.

@JHWelch
Copy link
Contributor Author

JHWelch commented Oct 16, 2025

Yeah, I suppose this is a six of one, half dozen of another. This is so buried in the core of MM that most folks probably aren't seeing it anyway.

It is just if you google "Subclass a method" you get some docs on Java, where this terminology seems most common, and mostly things about "how to call a subclass from a superclass".

If you google "Override a method" you get the Wikipedia page on the concept, and a number of articles for different languages.

I agree with @sdetweil that these comments could be completely overhauled to be more non-developer friendly, maybe include some more information a la the Core module File docs page. Or just link out to that. But think this is still a step in the right direction.

Copy link
Collaborator

@KristjanESPERANTO KristjanESPERANTO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "override" is an improvement over "subclass". Sure, the comments could be overhauled to be more beginner-friendly - but one can do that in a future PR. This is a step in the right direction.

@KristjanESPERANTO KristjanESPERANTO changed the title Update subclass language use override [weatherprovider] update subclass language use override Nov 3, 2025
@KristjanESPERANTO
Copy link
Collaborator

@sdetweil Can we merge this? 🙂

@KristjanESPERANTO
Copy link
Collaborator

Since it's just a minor thing and there was enough time to comment on it, I'll merge it now. @JHWelch, thank you for this contribution! 🙂 If anyone has better wordings, we can discuss it further here or in a new PR 🙂

@KristjanESPERANTO KristjanESPERANTO merged commit 53df20f into MagicMirrorOrg:develop Nov 13, 2025
9 checks passed
@sdetweil sdetweil mentioned this pull request Jan 1, 2026
sdetweil added a commit that referenced this pull request Jan 1, 2026
Thanks to: @Blackspirits, @Crazylegstoo, @jarnoml, @jboucly, @JHWelch,
@khassel, @KristjanESPERANTO, @rejas, @sdetweil, @xsorifc28

⚠️ This release needs nodejs version >=22.21.1 <23 || >=24

[Compare to previous Release
v2.33.0](v2.33.0...develop)

[core]
Prepare Release 2.34.0
(#3998)
dependency update + adjust Playwright setup + fix linter issue
(#3994)
demo with gif (#3995)
[core] fix: allow browser globals in config files
(#3992)
[core] fix: restore --ozone-platform=wayland flag for reliable Wayland
support (#3989)
[core] auto create release notes with every push on develop
(#3985)
[core] chore: simplify Wayland start script
(#3974)
[gitignore] restore the old Git behavior for the default modules
(#3968)
[core] configure cspell to check default modules only and fix typos
(#3955)
[gitignore] restoring the old Git behavior for the CSS directory
(#3954)
feat(core): add server:watch script with automatic restart on file
changes (#3920)
[check_config] refactor: improve error handling
(#3927)
refactor: replace express-ipfilter with lightweight custom middleware
(#3917)
refactor: replace module-alias dependency with internal alias resolver
(#3893)

[dependencies]

[chore] update dependencies and min. node version
(#3986)
[core] bump dependencies into december
(#3982)
Bump actions/checkout from 5 to 6
(#3972)
Update deps, unpin parse5
(#3934)
[core] Update deps and pin jsdom to v27.0.0
(#3925)
chore: update dependencies
(#3921)
update deps, exclude node v23
(#3916)
remove eslint warnings, add npm publish process to Collaboration.md
(#3913)
feat: add ESlint rule no-sparse-arrays for config check
(#3911)
chore: bump dependencies into october
(#3909)

[logging]

logger: add calling filename as prefix on server side
(#3926)
[logger] Add prefixes to most Log messages
(#3923)

[modules/alert]
Add new pt and pt-BR translations for Alert module and update global PT
strings (#3965)

[modules/calendar]
add checksum to test whether calendar event list changed
(#3988)
[calendar] fix: prevent excessive fetching on client reload and refactor
calendarfetcherutils.js
(#3976)
[calendar] refactor: migrate CalendarFetcher to ES6 class and improve
error handling (#3959)
[calendar] Show repeatingCountTitle only if yearDiff > 0
(#3949)
[tests] suppress debug logs in CI environment + improve calendar symbol
test stability (#3941)
[calendar] chore: remove requiresVersion: "2.1.0"
(#3932)
[calendar] test: remove "Recurring event per timezone" test
(#3929)

[modules/compliments]
[compliments] refactor: optimize loadComplimentFile method and add unit
tests (#3969)
fix: set compliments remote file minimum delay to 15 minutes
(#3970)
[compliments] fix: duplicate query param "?" in compliments module
refresh url (#3967)

[modules/newsfeed]
[newsfeed] fix header layout issue
(#3946)

[modules/weather]
[weatherprovider] update subclass language use override
(#3914)
[weather] fix wind-icon not showing in pirateweather
(#3957)
[weather] add error handling to weather fetch functions, including cors
(#3791)
remove deprecated ukmetoffice datapoint provider, cleanup .gitignore
(#3952)
fixes problems with daylight-saving-time in weather provider openmeteo
(#3931)
Fix for envcanada Provider to use updated Env Canada URL
(#3919)
[weather] feat: add configurable forecast date format option
(#3918)

[testing]
testing: update "Enforce Pull-Request Rules" workflow
(#3987)
[core] refactor: replace XMLHttpRequest with fetch and migrate e2e tests
to Playwright (#3950)
[test] replace node-libgpiod with serialport in electron-rebuild
workflow (#3945)
[ci] Add concurrency to automated tests workflow to cancel outdated runs
(#3943)
[tests] migrate from jest to vitest
(#3940)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Karsten Hassel <hassel@gmx.de>
Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com>
Co-authored-by: Ryan Williams <65094007+ryan-d-williams@users.noreply.github.com>
Co-authored-by: Veeck <github@veeck.de>
Co-authored-by: Bugsounet - Cédric <github@bugsounet.fr>
Co-authored-by: Marc Landis <dirk.rettschlag@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: HeikoGr <20295490+HeikoGr@users.noreply.github.com>
Co-authored-by: Pedro Lamas <pedrolamas@gmail.com>
Co-authored-by: veeck <gitkraken@veeck.de>
Co-authored-by: Magnus <34011212+MagMar94@users.noreply.github.com>
Co-authored-by: Ikko Eltociear Ashimine <eltociear@gmail.com>
Co-authored-by: DevIncomin <56730075+Developer-Incoming@users.noreply.github.com>
Co-authored-by: Nathan <n8nyoung@gmail.com>
Co-authored-by: mixasgr <mixasgr@users.noreply.github.com>
Co-authored-by: Savvas Adamtziloglou <savvas-gr@greeklug.gr>
Co-authored-by: Konstantinos <geraki@gmail.com>
Co-authored-by: OWL4C <124401812+OWL4C@users.noreply.github.com>
Co-authored-by: BugHaver <43462320+bughaver@users.noreply.github.com>
Co-authored-by: BugHaver <43462320+lsaadeh@users.noreply.github.com>
Co-authored-by: Koen Konst <koenspero@gmail.com>
Co-authored-by: Koen Konst <c.h.konst@avisi.nl>
Co-authored-by: dathbe <github@beffa.us>
Co-authored-by: Marcel <m-idler@users.noreply.github.com>
Co-authored-by: Kevin G. <crazylegstoo@gmail.com>
Co-authored-by: Jboucly <33218155+jboucly@users.noreply.github.com>
Co-authored-by: Jboucly <contact@jboucly.fr>
Co-authored-by: Jarno <54169345+jarnoml@users.noreply.github.com>
Co-authored-by: Jordan Welch <JordanHWelch@gmail.com>
Co-authored-by: Blackspirits <blackspirits@gmail.com>
Co-authored-by: Samed Ozdemir <samed@xsor.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants