Review sprint #3

[niemeyer]

It’s that time again!

We have quite a few pending PRs open ranging from major features all the way to trivial tweaks. To ensure these get addressed timely, we’ll have a more focused effort over the next few days to work through them. If you are an author or a reviewer, let’s please collaborate towards that.

Below we have the PRs which were open when the sprint started today, and perhaps a few important ones that were opened later and then integrated into the sprint. The notations used are described after the list.

For any required updates unrelated to status changes, please comment below. Status changes will be updated regularly and automatically.

Reviews

#3120: [WIP] interfaces/hooks: expose attrs to the interface API, snapctl enhancements (step #4) {w:_pz} #3260: cmd/snap: implement userd command as replacement for snapd-xdg-open {+:znm+_c+} (2.28) #3372: tests: add basic lxd test {m:n+_zs} #3398: env: set XDG_DATA_DIRS for wayland et.al. {+:mn+_zjc} (2.28) #3484: tests: add autopilot-introspection interface test {+:ns₊} #3502: snap-seccomp: add in-kernel bpf tests {m:jz_n} #3556: client,daemon,snap,store: add license field {+:n⁺_zm+} #3573: overlord: always try to get a serial, lazily on classic {p:mn} (2.28) #3616: cmd/snap-repair: check signatures of repairs from Next {p:nm} #3617: interfaces/builtin: use udev tagging more broadly {+:jn_mz+} (2.28) #3621: cmd/snap-{confine,update-ns}: apply mount profiles using snap-update-ns {z:w^j_nmp} (2.28) #3625: many: end-to-end support for the bare base snap {m:zn₊} #3635: tests: add out-of-the-box test suite {z:^sn_m} #3642: cmd/snap: get keys or root document {w:n_zp} #3697: docs: add PULL_REQUEST_TEMPLATE.md {m:zwpⁿ⁺} #3719: interfaces: add desktop and desktop-legacy {j:zn} (2.28) #3720: cmd,interfaces: add initial support for Solus {+:z^nj₊} #3727: daemon, snapstate: move ensureCore from daemon/api.go into snapstate.go {m:p} #3734: packaging: add debian-unstable {+:ⁿ_z+} #3748: many: fetch & cache remote snaps and sections; complete from there {c:_pz} #3750: snapstate: integration test for undoing a daemon restart on classic {m:_zw} #3755: try to reenable fedora spread tests {p:z} #3760: Allow snap-confine to be confined even with --disable-apparmor {+:} #3773: snap-repair: execute the repair and capture logs/status {m:wⁿ_p} #3777: snap-repair: implement basic snap-repair list (with --verbose) {m:zⁿ} #3779: snap-seccomp: remove use of x/net/bpf from tests {m:pw} #3780: many: configure store from state, reconfigure store at runtime {+:^c_p+} #3781: cmd/snap-repair: track and use a lower bound for the time for TLS checks {p:mzⁿ} #3795: daemon: let client decide whether to allow interactive auth via polkit {+:znm⁺_c} #3797: daemon: allow polkit authorisation to install/remove snaps {+:zⁿ_m+} #3804: cmd/snap-seccomp: support parsing ‘u:’ and ‘g:’ for username and groups {j:_z} #3805: interfaces/{default,account-control}: Use username/group instead of uid/gid {j:mc_z} #3807: cmd/snap-confine,packaging: import snapd-generated policy {z:m₊} #3810: interfaces/hooks: PlugData and SlotData wrappers {w:_zpm} #3812: interfaces: expose bluez interface on classic OS {+:zj} #3814: interfaces: enable partial apparmor support {z:jⁿ_m} #3815: wrappers: ensure bash completion snaps install on core {m:cz} (2.28) #3816: hooks: do not error out when hook is optional and no hook handler is registered {w:zm} (2.28) #3818: interfaces: fix network-manager plug {m:wz} (2.27) #3819: hooks: do not error when hook handler is not registered (2.27) {w:n} #3822: vendor: use old golang.org/x/crypto/ssh/terminal to build on powerpc again {m:cn} (2.28)

Stats

41 total → ( 1 waiting → 10 reviewing → 0 reviewed → 24 merged ) → 30 closed

Status of each PR:

• Waiting for review
• Open review
• Reviewed and approved
• Merged
• Closed without merging
• Blocked
• Critical
• Decaying
• Broken spread tests (click to see)
• (2.NN) Targets given milestone
• {.:…} Reviews from regulars (see below)

Regular reviewers

As an aid during the review process, developers participating in reviews regularly are marked with:

• {1:2³⁴₅}

Where the regular reviewer:

1. Submitted the PR
2. Approved the PR
3. Was asked to review the PR
4. Requested changes in PR
5. Commented on the PR

The following regular reviewers are currently being tracked:

Char	Reviewer
c	@chipaca
j	@jdstrand
m	@mvo
n	@niemeyer
p	@pedronis
s	@cachio
w	@pstolowski
z	@zyga-snapd
+	Exceptional

Note that those are the regular reviewers. We have many more contributors than this.

[pedronis]

afaict a char appears in position 3 also if that reviewer was requested specifically for a review and hasn’t given one yet

[niemeyer]

@pedronis Yeah, sorry. Fixed the description to say “has pending reviews”, which supports that as well.

[niemeyer]

@pedronis Ended up separating the case out so it’s more clear the difference between requested changes and a pending review. Did that by marking the latter in bold.

[niemeyer]

Thanks to everybody who helped making this review sprint a successful one. In pretty much a week we’ve reviewed and landed 30 out of 41 PRs, where two of the remaining ones are blocked for specific reasons, and all but one of the other ones are well on their way.

The third review sprint is now completed, and we expect to have another one in the coming weeks.