CodeReviews

Differences between revisions 3 and 17 (spanning 14 versions)
⇤ ← Revision 3 as of 2007-07-05 11:00:28 →
Size: 2292
Editor: i59F7228F
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 !!!'''
Line 11: 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 ||
Line 16: Line 16:
 * have either to be in `~ubuntu-core-dev` or `~ubuntu-dev` to be able to upload reviewed patches. Still efforts of interested new contributors in reviewing patches are appreciated,  * 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,
Line 23: Line 23:
The administrators of the team will review the list of bugs weekly and assign long standing bugs 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. 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.
Line 28: Line 32:
 * adhere to StableReleaseUpdates, FreezeExceptionProcess, Merge policy,
 * patches should build, built packages should install cleanly,		  * adhere to StableReleaseUpdates, FreezeExceptionProcess, SyncRequestProcess, Merge policy,
 * patches should apply, the resulting package should build and work correctly, built packages should install cleanly,
Line 31: Line 35:
 * 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 35: Line 48:
 * https://help.launchpad.net/PPAQuickStart
Line 39: Line 53:
  * 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)