Skip to content
This repository was archived by the owner on Feb 13, 2024. It is now read-only.

Conversation

MartinBriza
Copy link
Contributor

📒 Description

It's better to mark overriden virtual methods in child classes to avoid mixups when changing the underlying API or to just be sure that we're working with a virtual method in the child class.

🕶️ Types of changes

  • New feature (non-breaking change which adds functionality)

🔎 Review hints

  • Same as with most library overhaul PRs, warnings should disappear and the app should continue to compile and work the same on all platforms.

@MartinBriza MartinBriza force-pushed the fix/library-overhaul/rebased/06-mark-override-methods branch from 0f8be36 to eacb1cf Compare September 20, 2019 08:14
@NghiaTranUIT NghiaTranUIT self-requested a review October 7, 2019 08:59
@NghiaTranUIT
Copy link
Contributor

The change of this PR is toggl@eacb1cf and it works well in macOS / Windows by playing around the app.

However, this PR is children of the JSON PR, which has 1 issue. Therefore, I will approve but not merge until the host is properly addressed. 👍

Copy link
Contributor

@NghiaTranUIT NghiaTranUIT left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@MartinBriza MartinBriza force-pushed the fix/library-overhaul/rebased/06-mark-override-methods branch from eacb1cf to 61b6e53 Compare November 26, 2019 16:31
@MartinBriza MartinBriza merged commit 9b042cc into master Nov 27, 2019
@MartinBriza MartinBriza deleted the fix/library-overhaul/rebased/06-mark-override-methods branch November 27, 2019 09:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants