Skip to content

Monday, 24 July 2023

Akademy, KDE’s annual conference, recently took place in Thessaloniki, Greece. Lots of people were super excited about the prospect of getting GUI Testing off the ground based on the Selenium tech I built last year. Since KDE produces cross-platform applications an obvious question arose though…

What about Windows?

It’s surprisingly easy! Indeed the most time consuming part is probably getting your hands on a Windows Development Virtual Machine. Once you have a Windows installation we need to only spin up our toolchain and off we go. Here’s a handy command list:

\# Download and install WinAppDriver: https://github.com/microsoft/WinAppDriver/releases

winget install openjs.nodejs
winget install python.python.3.11

npm install --location=global appium
# restart terminal to apply PATH change
set-executionpolicy -scope currentuser remotesigned # allow script execution
appium driver install --source=npm appium-windows-driver

pip install appium-python-client
appium # start server, needs firewall exception on first start

Before we go further into the nitty gritty of testing on Windows I suggest you read the earlier blog post Selenium + AT-SPI = GUI Testing, since a lot of the concepts are the same regardless of platform.

First let us get our ducks in a row.

What Accerciser is to Linux is inspect.exe to Windows, namely an inspector tool for applications. You can find it in your Windows SDK folder %ProgramFiles(x86)%\Windows Kits\10\bin\10.0.22000.0\x64\inspect.exe or there abouts. Opening it greets you with this beauty:

Ignoring the verbosity for a moment we’ll note that it contains similar information to Accerciser on Linux, albeit in a more flat overview. Most importantly what is called the AutomationId is constructed from QObject objectNames, similar to the Accessible IDs on Linux. This is insofar interesting as it means we have a couple of avenues for cross-platform element locating - specifically we could match elements by their name (e.g. the text of a Label or Button), or more uniquely by their objectName-based ID (applicable to all QObjects).

For the purposes of this post we’ll do some trivial testing on Filelight and try to make it work for both Linux and Windows by using the element names. Relying on objectNames is more reliable but unfortunately requires some retrofitting in the source code. To avoid having to build Filelight on Windows we’ll work with what we have got: names. Let’s write our test. Don’t forget to install Filelight first :)

First thing, as always, is our setup boilerplate

#!/usr/bin/env python3

# SPDX-License-Identifier: MIT
# SPDX-FileCopyrightText: 2023 Harald Sitter <sitter@kde.org>

import unittest
import sys
from appium import webdriver
from appium.options.windows import WindowsOptions
from appium.options.common import AppiumOptions
from appium.webdriver.common.appiumby import AppiumBy
from selenium.webdriver.support.wait import WebDriverWait
from selenium.webdriver.support import expected\_conditions as EC

class SimpleFilelightTests(unittest.TestCase):
    @classmethod
    def setUpClass(self):
        options = WindowsOptions()
        options.app('KDEe.V.Filelight\_7vt06qxq7ptv8!KDEe.V.Filelight')
        self.driver = webdriver.Remote(
            command\_executor='http://127.0.0.1:4723',
            options=options)

    @classmethod
    def tearDownClass(self):
        self.driver.quit()

if \_\_name\_\_ == '\_\_main\_\_':
    unittest.main()

The only really interesting bit here is how we specify the application on Windows. Since Filelight is a store application we can start it by the Application User Model ID (AUMID) instead of a path; this is pretty much the same as starting by-desktop-file-id on Linux.

Now then. With the boilerplate out of the way we can write an incredibly simple test that simply switches pages a bit: If we click on ‘Scan Home Folder’ it should take us to the scan page and clicking there on ‘Go to Overview’ should take us back to the overview page.

    def test\_scan(self):
        self.driver.find\_element(by=AppiumBy.NAME, value="Scan Home Folder").click()
        overview = WebDriverWait(self.driver, 120).until(
            EC.element\_to\_be\_clickable((AppiumBy.NAME, "Go to Overview"))
        )
        overview.click()
        WebDriverWait(self.driver, 4).until(
            EC.element\_to\_be\_clickable((AppiumBy.NAME, "Scan Home Folder"))
        )

Cool. We now can test Filelight on Windows. Next we should try to make this test also work for Linux. Thankfully we only need to switch out our app name for a desktop file id.

    def setUpClass(self):
        if sys.platform == 'nt':
            options = WindowsOptions()
            options.app = 'KDEe.V.Filelight\_7vt06qxq7ptv8!KDEe.V.Filelight'
        else:
            options = AppiumOptions()
            options.set\_capability('app', 'org.kde.filelight.desktop')

Putting it all together we get our final test which runs on both Linux and Windows.

\# Windows
# start appium in a terminal
python .\\test.py

# Linux selenium-webdriver-at-spi-run ./test.py


The complete test code:

#!/usr/bin/env python3

SPDX-License-Identifier: MIT

SPDX-FileCopyrightText: 2023 Harald Sitter sitter@kde.org

import unittest import sys from appium import webdriver from appium.options.windows import WindowsOptions from appium.options.common import AppiumOptions from appium.webdriver.common.appiumby import AppiumBy from selenium.webdriver.support.wait import WebDriverWait from selenium.webdriver.support import expected_conditions as EC

class SimpleCalculatorTests(unittest.TestCase):

@classmethod
def setUpClass(self):
    if sys.platform == 'nt':
        options = WindowsOptions()
        options.app = 'KDEe.V.Filelight\_7vt06qxq7ptv8!KDEe.V.Filelight'
    else:
        options = AppiumOptions()
        options.set\_capability('app', 'org.kde.filelight.desktop')

    self.driver = webdriver.Remote(
        command\_executor='http://127.0.0.1:4723',
        options=options)

@classmethod
def tearDownClass(self):
    self.driver.quit()

def test\_scan(self):
    self.driver.find\_element(by=AppiumBy.NAME, value="Scan Home Folder").click()
    overview = WebDriverWait(self.driver, 120).until(
        EC.element\_to\_be\_clickable((AppiumBy.NAME, "Go to Overview"))
    )
    overview.click()
    WebDriverWait(self.driver, 4).until(
        EC.element\_to\_be\_clickable((AppiumBy.NAME, "Scan Home Folder"))
    )

if __name__ == ‘__main__’: unittest.main()


Discuss this blog post on [KDE Discuss](https://discuss.kde.org/t/writing-selenium-appium-tests-on-windows/3145).

Saturday, 22 July 2023

Monday, 17 July 2023

Make sure you commit anything you want to end up in the KDE Gear 23.08 releases to them

Dependency freeze is next July 20

The Feature Freeze and Beta is Thursday 27 of July.

More interesting dates  
  August 10: 23.08 RC (23.07.90) Tagging and Release
  August 17: 23.08 Tagging
  August 24: 23.08 Release

https://community.kde.org/Schedules/KDE_Gear_23.08_Schedule

Sunday, 16 July 2023

During this week Akademy 2023 is going on in Thessaloniki, Greece. It’s always awesome, to see many old friends and getting together with that amazing hacker community which is KDE.

There, me and Niccolò gave a talk about what;s happening in Plasma 6 and what will change, Noccolò on more visual things, about some changes we are cooking on the UI and on the visual themes. Here you can find a recording of the talk (alongside all the others of the day)

I talked more about the work I’ve bein doing in the Plasma shell during the last couple of months: code rafactors and how the API for writing plasmoids will change.

There were many things we were not quite happy about and now with the major release is the occasion for streamlining many things.

Now, It’s very important those changes are are well communicated, and easy to do for developes, because there are *a lot* of 3rd party plasmoids on the KDE store, which people are using and enjoying.

Let’s go trough the most important changes:

Dataengines

Dataengines were an API designed in early KDE 4 times, especially one of for our first offereings of Plasmoid API which was the pure JavaScript API, which existed long before the QML existed.

But now in a QML world, their API doesn’t really fit, instead is a much better fit having a QML extension which offers classes with all the needed properties, data models and signals that provide access to the needed data, such as tasks, notifications etc.

Dataengines are now deprecated and moved into a separed library, called “plasma5support” which will be still available for the time being, but consider porting away from it as we plan to eventually drop it.

Base Plasmoid API

The way plasmoids are declared in QML changed a bit: we used to have a magical “plasmoid” context property available from anywhere. This was an instance of a QQuickItem which was both the item where all the plasmoid contents were *and* a wrapper for some of the api for the central plasmoid object: the C++ class Plasma::Applet.

Now the access to plasmoid is an attahced property, the (uppercase) “Plasmoid”, which is directly the access to the instance of the central Plasma::Applet, without an in-between wrapper anymore.

The central QQuickItem is now called “PlasmoidItem”, and must be the root item of the plasmoid, just alike ApplicationWindow is for applications.

PlasmoidItem will have the purely graphical properties, such as the “compactRepresentation” or “fullRepresentation”

Here is a very minimal example of a plasmoid main file under plasma6:

import org.kde.plasma.plasmoid 2.0
PlasmoidItem {
    Plasmoid.title: i18n("hello")
    fullRepresentation: Item {....}
}

Actions

Plasmoids can export actions to their right mouse button menu, such as “mute” for the mixer plasmoid and so on.

In Plasma 5 we had an imperative API to add those actions, which was again coming from that old pure JS API, which really looked a bit out of tune in QML. In Plasma 6 the API has been replaced with a completely declarative API, in this form:

PlasmoidItem {
    Plasmoid.contextualActions: [
        PlasmaCore.Action {
            text: i18n("Foo")
            icon.name: "back"
            onTriggered: {...}
        },
        PlasmaCore.Action {
             ...
        }
    ]
}

PlasmaCore.Action is actually a binding to QAction (not the internal QML action type), so that it can be shared between C++ and QML easily

SVG theming

Plasma Themes don’t really have changed for now (and you can expect any old theme from the store to keep working), but the C++ and QML API for them has been moved to a standalone framework called KSvg. Plasma Svgs have quite some interesting features over the pure QtSvg API, such as disk caching of the rendered images, stylesheet recoloring to system colors and the 9 patch rectangular stretched images of FrameSvg.

Some applications were interested in using that, but couldn’t due to the long dependency chain of plasma-framework, so now they can be used as a much more manageable compact framework, offering both the usual C++, QPainter based api and QML bindings.

import org.kde.ksvg 1.0 as KSvg
FrameSvg {
    imagePath: "widgets/background"
}

Kirigami all the way down

Designing Kirigami in the beginning we lifted two concept from the Plasma API (which again we couldn’t use directly due to the dependency chain) Theme and Units

Theme gives access to the named system colors, and Units to standard spacing and animation durations.

Over the years the Kirigami version got way more advanced then the Plasma version, and having this code duplication didn’t make much more sense, to in Plasma6 whenever referring to a named color or an unit, the Kirigami version should be used, as the Plasma version is going away.

import org.kde.kirigami 2.20 as Kirigami
RowLayout {
    spacing: Kirigami.Units.smallSpacing
    Rectangle {
        color: Kirigami.Theme.backgroundColor
        bordere.color: Kirigami.Theme.textColor
    }
}

Wednesday, 12 July 2023

Today we are announcing the availability of the minor patch release 2.10.1. This release contains minor improvements and bug fixes only. The fixes are distributed over many different areas of the application and we recommend everybody update to this patch release which is available from our download page.

The full list of fixes included in this patch release are as follows:

  • Support markdown library discount version 3
  • Improve Vector BLF dependency (git download must be enabled if needed)
  • Correctly use system header of system QXlsx (BUG 468651)
  • Fix group separator problem in formulas (BUG 468098)
  • Improve log scales (auto scaling and tick number)
  • Improve auto scale (Issue #536)
  • Fix limits when changing scales (Issue #446)
  • Use system liborigin headers if linking against system liborigin (BUG 469367)
  • Properly import UTF8 encoded data (BUG 470338)
  • Do not clear the undo history when saving the project (BUG 470727)
  • Properly react on orientation changes in the worksheet properties explorer
  • In the collections of example projects, color maps and data sets also allow searching for sub-strings and make the search case-insensitive
  • Properly set the size of the worksheet in the presenter mode if “use view size” is used
  • Properly save and load the property “visible” for box and bar plots in the project file
  • Fix copy&paste and duplication of box and bar plots
  • Fix issues with loading object templates (BUG 470003)
  • Fix crash when loading projects with reference ranges
  • .xlsx import corrections:
    • fix crash importing empty cells
    • support datetime import (Issue #531)
  • Properly set the initial properties of the reference line, like line width, etc. (Issue #580)
  • Properly show the initial value of the property “visible” for the reference range (Issue #582)
  • React to Delete and Backspace keys to delete selected cells in spreadsheet columns (Issue #596)
  • Update the plot legend on column name changes used in box and bar plots (Issue #597)
  • Fix the positioning of values labels for horizontal bar plots (Issue #599)
  • Initialize the parameters for the baseline subtraction with reasonable values on first startup and improve the appearance of the preview plot

We are also working on the new features and improvements that will arrive in the next 2.11 release. This release will become available in the coming months. More on this in the next blog posts. Stay tuned!

We're happy to announce the new release 5.11.0 of KPhotoAlbum, the KDE photo management program!

Most notably, this release can be built against Exiv2 0.28, which introduced some breaking changes. Older versions are still supported as before.

Other things that have been changed and fixed (as listed in the ChangeLog) are:


Changed

  • Recalculate Checksums in the Maintenance menu and Refresh Selected Thumbnails in the thumbnail context menu have been unified to do exactly the same.
  • Simplified logging categories: kphotoalbum.XMLDB was merged into kphotoalbum.DB

Fixed

  • Fix issue where non-empty time units in the date bar were incorrectly greyed out (#467903)
  • Fix bug with the date bar showing and selecting incorrect date ranges (#468045)
  • Fix crash when the annotation dialog is opened from the viewer window and the viewer is closed before the annotation dialog (#470889)
  • Fix inconsistent UI where menu actions would not immediately be updated to reflect a change (#472109, #472113)

The list of contributors is quite short this time, it was only Johannes and me ;-) Anyway, thanks to everybody working on KPA in any way, to everybody having contributed in the past and for all future work!

Have a lot of fun with KPhotoAlbum 5.11.0 :-)

— Tobias

Monday, 10 July 2023

digiKam 8.1.0 Similarity Search Engine Dear digiKam fans and users,

After five months of active maintenance and long bugs triage, the digiKam team is proud to present version 8.1.0 of its open source digital photo manager.

See below the list of most important features coming with this release.

  • Print Creator: Add 4 new templates for 6.8 inches photo paper.
  • General : Improve usability of Image Properties sidebar tab.
  • Libraw : Update to snapshot 2023-05-14
  • Bundles : Update Exiv2 to last 0.28 release
  • Bundles : Update KF5 framework to last 5.106
  • Bundles : Includes Breeze widgets style in MacOS package to render properly GUI contents.
  • Tags : Add possibility to remove all face tags from selected items.
  • Tags : Add possibility to remove all tags from selected items except face tags.
  • Similarity : Add usability improvements about reference images while searching for duplicates images.

This version arrives with a long review of bugzilla entries. Long time bugs present in older version have been fixed and we spare a lots of time to contact users to validate changes in pre-release to confirm fixes before to deploy the program in production.

And it can be done easily, ackshually.

But what is that all about?

The problem

It has been a longstanding complaint that the ~/.config/ directory on Linux systems can get riddled with configuration files. This is the case with KDE software as well.

My idea is that we should be putting those into subdirectories inside ~/.config/.

The Freedesktop XDG Base Directory specification generally only states that standard configuration files should go under XDG_CONFIG_DIRS. Dump them there and you’re gold. So it’s not wrong to just fill the ~/.config/ directory with them.

Friday, 7 July 2023

Bundle Creator

Caution: Technical Jargon Zone!

If you had been following my earlier blog posts, you would know that I rarely include any code in them. My focus has primarily been on explaining how things work rather than delving into the specifics of how I implemented them. But this time I will be taking a deeper dive into the code, so in case you want to skip code today, you better not start reading this. ;)

This blog post has been a bit of a learning exercise for me as I pushed myself to learn UML diagrams and study a few design patterns in Qt. Learning Qt itself has been a challenge, though I doubt I can barely say that I have learnt it - I think it’s safe to say that I have just got more comfortable not understanding most things in Qt and trying to understand the parts that concern me. Now that I’m a teeny tiny bit wiser, I feel learning Object-Oriented Programming with C++, and a few design patterns prior to learning Qt would have been a better idea. Things (read classes) make a lot more sense once you understand the core design patterns.

Bundle Creator Wizard

The plan was to split the bundle creator into four main components, each having a single responsibility (Single Responsibility Principle!). DlgCreateBundle is the main class for the Bundle Creator. Notice how it has all functions related to putting the resources, tags and metadata in the bundle.

Similarly, all the code regarding resource choosing is present in PageResourceChooser(well not all, some of it in WdgResourcePreview), PageTagChooser(and WdgTagPreview) deals with the bundle’s tags, and all the metadata logic is present in PageMetaDataInfo. These wizard pages are completely independent of each other. There is, however, a message passing between PageBundleSaver and the other wizard pages which I will discuss later.

Resource Item Viewer

The Bundle Creator’s Resource Item Viewer now shares the same user interface as the one used by the Resource Manager in Krita. However, in order to not upset existing users of Krita, a new View Mode Button has been added so that users can switch between grid view and list view as per their preference.

The WdgResourcePreview class only deals with the left half of the Bundle Creator and the Resource Manager. That said, it loads the resources from the Resource Database onto the viewer, and displays resources as filtered by text or tag. However, all the code related to what happens when a resource item is clicked is dealt within the PageResourceChooser class for the Bundle Creator and the DlgResourceManager for the Resource Manager.

To manipulate the working of the right half of the Resource Chooser Page, one would need to make modifications to PageResourceChooser. And even though the left and right halves of the Resource Chooser page look fairly identical, it is important to note that the left half is built upon a QListView (KisResourceItemListView) and the right one on a QListWidget (KisResourceItemListWidget). This is because the left half loads the data directly from the Resource Database, using KisResourceModel. And the right half provides a view of the resource items selected by the user. It does use KisResourceModel for fetching the icon and name of the relevant item, but it doesn’t use the model directly.

This is really how each class mentioned above looks like.

Common UI

Qt’s Model View Architecture in Bundle Creator

Similarly to MVC, Qt’s Model/view design pattern is essentially separated into three components: Model, View and Delegate.

Instead of utilizing controller classes, Qt’s view handles data updating through delegates. It serves two primary objectives: firstly, aiding the view in rendering each value, and secondly, facilitating user-initiated changes. As a result, the controller’s responsibilities have merged with the view, as the view now assumes some of the tasks traditionally assigned to the controller through Qt’s delegate mechanism.

The KisResourceModel, KisTagModel, KisStorageModel act as the models for the QComboBox-es in the Bundle Creator(and Resource Manager). The KisTagFilterResourceProxyModel is built on top of the KisResourceModel and KisTagModel, and serves as a model for the KisResourceItemView which displays the list of available resources. And the KisResourceItemDelegate renders the items of data. When an item is edited, the delegate communicates with the model directly using model indexes.

ModelView
KisResourceModelQComboBox
KisTagModelQComboBox
KisStorageModelQComboBox
KisTagFilterResourceProxyModelKisResourceItemView

Signal Slot Mechanism in Bundle Creator

Very classic, but just a rough sketch showing how the wizard pages communicate with one another. This connection helps to update the summary in PageBundleSaver whenever the selected list of resources or tags changes.

A bit about the Tag Chooser

This is something I have been working on last week. The Tag Chooser page is updated to look similar to the Resource Manager’s tag section. The available tags are displayed using KisTagLabel and the selected ones are displayed(and selected) using KisTagSelectionWidget. In both the cases, the KisTagModel serves as the underlying model.

Merge Request

My merge request can be viewed here.

Important Commits

Plans post Mid-Term Evaluation

Post midterm, I would be working on adding the feature of editing bundles in Krita, which will allow artists to add and delete components from existing bundles, so that they won’t have to go through the process of creating a bundle from scratch whenever they want to make some changes. I’ve created a post on Krita Artists Forum to better understand the preferences of artists regarding bundle editing. Feel free to drop a comment if you want to talk about it! :D


This time a drawing on paper art since I have exhausted my collection of art I made using Krita - serves as a reminder that I should do this more often. :)

Hand Drawn

I recently stumbled upon this excellent 2014 article about patch review by Sage Sharp: The Gentle Art of Patch Review.

I highly recommend reading the whole article, but my main takeaway is that when reviewing code, one should follow three phases:

  1. Answer the question: "Good or bad idea?"
  2. "Is this architecturally sound?": are the changes done in a way that makes sense given the existing project architecture?
  3. only then, can you go all in on nitpicking on naming, spelling mistakes, or ask for the commit history to be cleaned.

I do quite a lot of reviews at work. Sage article made me realize I am often guilty of jumping directly to #3. I have been reflecting on why that happens, and I concluded that it's because it's the path of least resistance.

When I receive a 10,000 line patchset, with 23 commits stepping on each other and no description, the temptation is high to skip phase 1 and 2 and instead say to myself: "I am just going to review the whole patch one line at a time, suggesting new names and micro-optimizations so that it does not look like I clicked on Approve while looking elsewhere".

Since jumping directly to phase 3 is the path of least resistance, when preparing code for review, you should make what you can to reduce the resistance of phase 1 and 2.

Phase 1: Is this a good or bad idea?

For this phase, the place to make review easier is in the description of your patchset: that is the pull request description on GitHub or the body of the mail presenting the patchset in a mail-based workflow.

It does not have to be extra long, just explain in one or two sentences the problem fixed by the patchset, or the new feature it adds... If the changes are significant, then hopefully you discussed them before diving full in: maybe there is an opened issue in the bug tracker, or it was discussed by chat or on a mailing-list. If so, add a link to it to your description.

Phase 2: Are the changes architecturally sound?

For this phase, there are 3 places to make review easier: patchset description, high-level comments and commit history.

Patchset description

The patchset description should not paraphrase the patchset, but it should give a high-level overview of the changes.

Let's say as part of your patchset, you moved class Foo from file A to file B. If a reviewer goes directly through the changes (as in, jumps directly to Phase 3), they will have to infer that given that class Foo was removed from A, and a very similar (but maybe not identical!) class Foo was added to B, then it means the patchset moved class Foo from A to B. By listing this move in your description, not only do you save the reviewer the pain of having to infer the move, but you also get the chance to explain why you moved this class.

This can be hard. Maybe you do not like writing so this is chore. Well, being able to write prose is a very useful skill, even for the developers! Stepping out of your comfort zone is difficult, but the more you practice, the better your writing will be, the less it will become a shore.

Another reason this can be hard is because you need to write this in English and you think your English is not good enough. As a friend once told me: "The language of Open-Source is broken English", so don't be afraid of this. A broken English description is much more helpful than no description at all, and if someone criticizes your English, well... shame on them! You did the work! And again, practice will improve your English writing too.

High-level comments

Some coding guidelines require every function, every constant, every class, every module to have documentation. I think those are misguided and often lead to Captain Obvious comments like that:

def activate_rear_engine():
    """Activate the rear engine"""
    ...

Thank you, Captain Obvious

On the other hand, having a comment attached to a new class or a module is very helpful, especially if it explains how the class works with other classes. Again, keep it high-level.

Commit history

Ideally changes should be small, but it's not always possible. If the changes are large, chances are they can be split in multiple smaller changes. It is a lot easier to review a large patchset if one can go through it commit by commit.

Except... this is a double-edged sword! It is only easier to review if each commit makes sense in isolation and if the set of commits "tell a story". If a commit adds a method to a class and the next commit renames or removes it with a "Oups, that was not a good idea" commit message, then it actually takes longer and is more painful to review commits individually than just reviewing the final state.

This is easier said than done, though: when you are working on your changes, it's common to hit road blocks, change directions, or make unrelated changes. The commit story usually won't be straight and easy to read on your first try.

You know what? That's fine! Just like the first draft of a novel writer is not perfect, so is your commit story. And just like the novel writer, you can edit your story to make it easier to read. Assuming you work with a VCS which lets you rewrite history like Git, once you are done with your first draft, it's time to warm up git rebase!

Again, this takes practice to get right, but here are a few tips to get started:

  • Splitting commits is more complicated than joining them, so it's simpler to create smaller commits and join them later.
  • You can create partial commits using git add -p or git gui.
  • Before starting a large history rewriting session, create a quick backup branch with git branch backup. This serves two purposes:
    1. If things go wrong, it's easy to revert to a known good state using git reset --hard backup (you can also use git reflog to do that, but I find this approach simpler).
    2. When you are done with your rewrite, you can run git diff backup to check if the end result is similar enough.
  • Maybe some of the changes are unrelated? If it's possible, move them to a separate branch and create a separate patch review for them. The smaller the changes, the easier it is to review.

Addressing review issues

This part depends on the policy of the project. Some projects prefer if you force-push the fixed patchset as you address issues spotted during review, so that the commit history always looks like what will be committed. Other projects have a no-force-push policy, they will ask you to push fixup commits. This often happens on forges which lack the ability to compare patch-sets (Unfortunately, GitHub is one of them, GitLab is much better in that regard).

If you get asked to do fixup commits, please make sure the history is squashed into a readable and coherent history before the patchset is merged. This greatly simplifies the lives of future developers if they have to revisit the past.

Wrapping up

To wrap up, to make the life of your reviewer easier (and thus make the review faster and/or more insightful):

  • Provide a high-level description of your changes, refer any prior discussion about it.
  • Without going all-in and adding comments everywhere, do provide high-level comments for new classes or modules.
  • Try to keep your commit history easy to read.

You may also find out during this preparation process that some of your changes are not optimal, or a bug slipped in. When this happens, take the time to fix these issues before posting your patchset for review. In a sense, this process makes you the first reviewer of your code. And the cleaner your patchset, the faster the review!