CodeReviews

Differences between revisions 1 and 17 (spanning 16 versions)
⇤ ← Revision 1 as of 2007-07-03 11:11:01 →
Size: 1000
Editor: i59F76E35
Comment:
← Revision 17 as of 2007-10-31 17:09:37 → ⇥
Size: 5073
Editor: 12
Comment:
Deletions are marked like this. Additions are marked like this.
Line 1: Line 1:
'''¡¡¡ This is Work In Progress !!!''' == Team Purpose ==
Line 9: Line 9:
If you are a member of the UbuntuDevelopers, it's highly appreciated if you join the team and help out: https://launchpad.net/~ubuntu-code-reviewers If you are a member of the UbuntuDevelopers, it's highly appreciated if you join the team and help out:
|| '''main''' / '''restricted''' || https://launchpad.net/~ubuntu-main-sponsors || https://lists.ubuntu.com/mailman/listinfo/ubuntu-main-sponsors ||
|| '''universe''' / '''multiverse''' || https://launchpad.net/~ubuntu-universe-sponsors || https://lists.ubuntu.com/mailman/listinfo/ubuntu-universe-sponsors ||

== Workflow ==

Members of the team
 * have either to be in `~ubuntu-core-dev` (for `main`/`restricted`) or `~ubuntu-dev` (for `universe`/`multiverse`) to be able to upload reviewed patches. Still efforts of interested new contributors in reviewing patches are appreciated,
 * will triage bugs (in the bug lists mentioned above) weekly,
 * pick bugs they're interested (based on their area of expertise and preference),
 * assign bugs where a review might take longer to themselves to avoid duplication of work.
 
If the patch was deemed not to be good enough yet, the bug will be re-assigned to the patch author and set to `In Progress`. The preferred way of closing bugs is ClosingBugsFromChangelog.

The administrators of the team will
 * review the list of bugs weekly,
 * assign long standing bugs (>= 2 weeks) to team members who might be up to the job.

Possible conflicts and problems will be discussed in the last minutes of the weekly Distro Team meeting.

== Review ==

As a reviewer you should make sure the following rules are observed:
 * adhere to StableReleaseUpdates, FreezeExceptionProcess, SyncRequestProcess, Merge policy,
 * patches should apply, the resulting package should build and work correctly, built packages should install cleanly,
 * don't hesitate to let the patch go through several review iterations, if you're unsure or unhappy about it.
 * sync requests don't needs sponsoring or uploads; if you're sufficiently happy with the sync request of a non team member, simply
  * mark it as confirmed,
  * add your ''ACK'' in a comment,
  * unsubscribe the sponsors team,
  * subscribe `ubuntu-archive` to the bug.
 * keep the name of the patch author in `debian/changelog` and use `-k<keyid>` to sign the package with your key.
 * make sure to ask the patch author to submit the patch to Debian and/or Upstream.

http://people.ubuntu.com/~dholbach/sponsoring/ has an overview of the bugs in those lists with assignees.
Line 14: Line 48:
 * https://help.launchpad.net/PPAQuickStart
Line 17: Line 52:
  * SponsorshipProcess
  * SyncRequestProcess
 * http://people.debian.org/~mpalmer/sponsorship_checklist.html
 * http://ftp-master.debian.org/REJECT-FAQ.html
 * [http://www.debian.org/doc/manuals/maint-guide/index.en.html Debian New Maintainer's Guide]
 * [http://www.debian.org/doc/debian-policy/ Debian Policy]

[[Anchor(Tips)]]
== Review Tips ==

=== Source ===

 * Review debdiff of version in the archive with the proposed package.
 * Check the release target in `debian/changelog`.
 * If it's a new upstream, check if the `.orig.tar.gz` is the same as the upstream one. (`md5sum(1)`). The reasons for differing tarballs must be in `debian/changelog`.
 * Use `interdiff(1)` (`patchutils` package) against the `.diff.gz` and `debdiff`(1) ({{{devscripts}}} package) against your test-built binary packages to examine what you've changed (and ensure it tallies with what you expected to change).
  * Use filterdiff to exclude generated or annoying parts
 * Check if `debian/copyright` is correct, accurate and complete.
  * See http://lists.debian.org/debian-devel-announce/2003/12/msg00007.html and http://lists.debian.org/debian-legal/2003/12/msg00194.html
  * If a package uses several licences, list which file is under which licence
  * Always make sure it is clean, who the copyright holder is, and the year of copyright
 * Check for unuseful comments (e.g. autogenerated # dh_X in debian/rules) in scripts


=== Build ===
 * Check if the clean target works properly (build twice).
 * NEW packages should be tried to build in pbuilder (to verify sufficient Build-Depends).
 * Run `dpkg -c` on the built package or `debc` in the source tree to see if everything installed to the right place.
 * Run lintian on the `<package>_<version>_<arch>.changes` file.

Team Purpose

A team of UbuntuDevelopers has agreed on doing periodical reviews of code. To accelerate SponsorshipProcess members of the team will check

If you are a member of the UbuntuDevelopers, it's highly appreciated if you join the team and help out:

Workflow

Members of the team

  • have either to be in ~ubuntu-core-dev (for main/restricted) or ~ubuntu-dev (for universe/multiverse) to be able to upload reviewed patches. Still efforts of interested new contributors in reviewing patches are appreciated,

  • will triage bugs (in the bug lists mentioned above) weekly,
  • pick bugs they're interested (based on their area of expertise and preference),
  • assign bugs where a review might take longer to themselves to avoid duplication of work.

If the patch was deemed not to be good enough yet, the bug will be re-assigned to the patch author and set to In Progress. The preferred way of closing bugs is ClosingBugsFromChangelog.

The administrators of the team will

  • review the list of bugs weekly,

  • assign long standing bugs (>= 2 weeks) to team members who might be up to the job.

Possible conflicts and problems will be discussed in the last minutes of the weekly Distro Team meeting.

Review

As a reviewer you should make sure the following rules are observed:

  • adhere to StableReleaseUpdates, FreezeExceptionProcess, SyncRequestProcess, Merge policy,

  • patches should apply, the resulting package should build and work correctly, built packages should install cleanly,
  • don't hesitate to let the patch go through several review iterations, if you're unsure or unhappy about it.
  • sync requests don't needs sponsoring or uploads; if you're sufficiently happy with the sync request of a non team member, simply
    • mark it as confirmed,

    • add your ACK in a comment,

    • unsubscribe the sponsors team,

    • subscribe ubuntu-archive to the bug.

  • keep the name of the patch author in debian/changelog and use -k<keyid> to sign the package with your key.

  • make sure to ask the patch author to submit the patch to Debian and/or Upstream.

http://people.ubuntu.com/~dholbach/sponsoring/ has an overview of the bugs in those lists with assignees.

References

Anchor(Tips)

Review Tips

Source

  • Review debdiff of version in the archive with the proposed package.

  • Check the release target in debian/changelog.

  • If it's a new upstream, check if the .orig.tar.gz is the same as the upstream one. (md5sum(1)). The reasons for differing tarballs must be in debian/changelog.

  • Use interdiff(1) (patchutils package) against the .diff.gz and debdiff(1) (devscripts package) against your test-built binary packages to examine what you've changed (and ensure it tallies with what you expected to change).

    • Use filterdiff to exclude generated or annoying parts

  • Check if debian/copyright is correct, accurate and complete.

  • Check for unuseful comments (e.g. autogenerated # dh_X in debian/rules) in scripts

Build

  • Check if the clean target works properly (build twice).
  • NEW packages should be tried to build in pbuilder (to verify sufficient Build-Depends).

  • Run dpkg -c on the built package or debc in the source tree to see if everything installed to the right place.

  • Run lintian on the <package>_<version>_<arch>.changes file.

Go back to [:UbuntuDevelopment].BR CategoryProcess

UbuntuDevelopment/CodeReviews (last edited 2023-08-21 12:01:15 by kewisch)