[GNOME] Providing feedback on patches and branches

awesome advert:

Did you know that GXml has a mailing list now?  gxml-list at gnome dot org!


Finding time to review


A week before GUADEC, the spectacular Daniel Espinosa let me know of work he's doing on GXml serialization in a new branch.  Sadly, it's taken me three weeks to provide adequate feedback.

There were three obstacles to this. 

  1. having my HP tc4400 laptop (her name was clarity, after Claire Danes) die shortly thereafter.  

  2. going to GUADEC (there was a lot of new work to do while there!)

  3. my thesis (sadly, I don't have my summer totally free)

  4. other GSOC goals

I feel like such a long delay, even if the causes seem reasonable, is terrible for encouraging good contribution's like Daniel.  I'm lucky Daniel is currently  motivated and has goals.

Doing the review

Because the new work is on a branch, and the changes are a bit extensive, it was a bit challenging keeping track of all the changes.  That's opposed to a set of smaller patches or contained changes, which I might be able to analyse in parts.  Because of cross-file changes and some code-reorganisation, I ended up using emacs ediff and some hand-editing to ease my comparison.  Sometimes changes look bigger on the outside, until you realise that fundamentally a lot of the new logic is the same.

I finally ended up spending about 5 hours reviewing it, which feels a little excessive, and I hope I get better at it.  As a graduate teaching assistant at my university, I'm used to reviewing students' code (which is more predictable and less complex).  I think one problem is that I didn't want to miss anything; I feel as though a quicker and sloppier review might actually be preferable for its quicker return time, than having a thorough and careful one that takes forever to find the time to start.

Providing feedback

One of the hardest parts is providing meaningful and friendly resistance to changes.  I want to make sure that changes are safe (smaller and more precise ones are preferable to minimise new bugs) and necessary (changing APIs without a clear benefit is painful for existing developers), but I don't want to overly discourage a submitter.

I tried to ask specific questions about the motivations behind certain changes, and tried to propose smaller changes that could accomplish the same purpose.

Hopefully I will prove more responsive in the future and Daniel's work will improve GXml's serialization support. :D

Advice from you

I'd appreciate any advice you have for reviewing code and encouraging submitters.  I heard that cursing at contributors works well for some well-known maintainers, but I don't think GXml is quite popular enough to afford that level of abuse. :)

Keine Kommentare:

Kommentar veröffentlichen

Dieses Blog durchsuchen


#Technology #GNOME gnome gxml fedora bugs linux vala google #General firefox security gsoc GUADEC android bug xml fedora 18 javascript libxml2 programming web blogger encryption fedora 17 gdom git emacs libgdata memory mozilla open source serialisation upgrade web development API Spain containers design evolution fedora 16 fedora 20 fedora 22 fedup file systems friends future glib gnome shell internet luks music performance phone photos php podman preupgrade tablet testing typescript yum #Microblog Network Manager adb apache art automation bash brno catastrophe css data loss debian debugging deja-dup disaster docker emusic errors ext4 facebook fedora 19 gee gir gitlab gitorious gmail gobject google talk google+ gtk html libxml mail microsoft mtp mysql namespaces nautilus nextcloud owncloud picasaweb pitivi ptp python raspberry pi resizing rpm school selinux signal sms speech dispatcher systemd technology texting time management uoguelph usability video web design youtube #Tech Air Canada C Electron Element Empathy Europe GError GNOME 3 GNOME Files Go Google Play Music Grimes IRC Mac OS X Mario Kart Memento Nintendo Nintendo Switch PEAP Selenium Splatoon UI VPN Xiki accessibility advertising ai albums anaconda anonymity apple ask asus eee top automake autonomous automobiles b43 backup battery berlin bit rot broadcom browsers browsing canada canadian english cars chrome clarity comments communication compiler complaints computer computers configuration console constructive criticism cron cropping customisation dataloss dconf debug symbols design patterns desktop summit development discoverability distribution diy dnf documentation drm duplicity e-mail efficiency email english environment estate experimenting ext3 fedora 11 festival file formats firejail flac flatpak forgottotagit freedom friendship fuse galaxy nexus galton gay rights gdb german germany gimp gio gjs gnome software gnome-control-center google assistant google calendar google chrome google hangouts google reader gqe graphviz growth gtest gtg gvfs gvfs metadata hard drive hard drives hardware help hp humour ide identity instagram installation instant messaging integration intel interactivity introspection jabber java java 13 jobs kernel keyboard language language servers languages law learning lenovo letsencrypt libreoffice librpm life livecd liveusb login lsp macbook maintainership mariadb mario matrix memory leaks messaging mounting mouse netflix new zealand node nodelist numix obama oci ogg oggenc oh the humanity open open standards openoffice optimisation org-mode organisation package management packagekit paint shedding parallelism pdo perl pipelight privacy productivity progress progressive web apps pumpkin pwa pyright quality recursion redhat refactoring repairs report rhythmbox rust sandboxes scheduling screenshots self-navigating car shell sleep smartphones software software engineering speed sql ssd synergy tabs test tests themes thesis tracker travel triumf turtles tv tweak twist typing university update usb user experience valadoc video editing volunteering vpnc waf warm wayland weather web apps website wifi wiki wireless wishes work xinput xmpp xorg xpath
Powered by Blogger.