Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1517)

Issue 5132048: code review 5132048: image: spin off a new color package out of the image pa... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 7 months ago by nigeltao
Modified:
12 years, 7 months ago
Reviewers:
CC:
r, bsiegert, golang-dev
Visibility:
Public.

Description

image: spin off a new color package out of the image package. The spin-off renames some types. The new names are simply better: image.Color -> color.Color image.ColorModel -> color.Model image.ColorModelFunc -> color.ModelFunc image.PalettedColorModel -> color.Palette image.RGBAColor -> color.RGBA image.RGBAColorModel -> color.RGBAModel image.RGBA64Color -> color.RGBA64 image.RGBA64ColorModel -> color.RGBA64Model (similarly for NRGBAColor, GrayColorModel, etc) The image.ColorImage type stays in the image package, but is renamed: image.ColorImage -> image.Uniform The image.Image implementations (image.RGBA, image.RGBA64, image.NRGBA, image.Alpha, etc) do not change their name, and gain a nice symmetry: an image.RGBA is an image of color.RGBA, etc. The image.Black, image.Opaque uniform images remain unchanged (although their type is renamed from image.ColorImage to image.Uniform). The corresponding color types (color.Black, color.Opaque, etc) are new. Nothing in the image/ycbcr is renamed yet. The ycbcr.YCbCrColor and ycbcr.YCbCrImage types will eventually migrate to color.YCbCr and image.YCbCr, but that will be a separate CL.

Patch Set 1 #

Patch Set 2 : diff -r 51de42dbe065 https://go.googlecode.com/hg/ #

Total comments: 1

Patch Set 3 : diff -r 087a4bd61f20 https://go.googlecode.com/hg/ #

Patch Set 4 : diff -r 087a4bd61f20 https://go.googlecode.com/hg/ #

Total comments: 3

Patch Set 5 : diff -r 087a4bd61f20 https://go.googlecode.com/hg/ #

Patch Set 6 : diff -r 087a4bd61f20 https://go.googlecode.com/hg/ #

Patch Set 7 : diff -r e144a6dec55e https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+821 lines, -328 lines) Patch
M src/cmd/gofix/Makefile View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
A src/cmd/gofix/imagecolor.go View 1 2 3 4 5 1 chunk +87 lines, -0 lines 0 comments Download
A src/cmd/gofix/imagecolor_test.go View 1 2 3 4 1 chunk +126 lines, -0 lines 0 comments Download
M src/cmd/gofix/imagenew_test.go View 1 2 2 chunks +2 lines, -4 lines 0 comments Download
M src/pkg/Makefile View 1 2 3 4 2 chunks +2 lines, -0 lines 0 comments Download
M src/pkg/image/Makefile View 1 1 chunk +0 lines, -1 line 0 comments Download
M src/pkg/image/bmp/reader.go View 1 2 3 4 5 chunks +6 lines, -5 lines 0 comments Download
A src/pkg/image/color/Makefile View 1 2 3 4 1 chunk +11 lines, -0 lines 0 comments Download
A src/pkg/image/color/color.go View 1 2 3 4 1 chunk +293 lines, -0 lines 0 comments Download
M src/pkg/image/decode_test.go View 1 2 3 4 2 chunks +2 lines, -1 line 0 comments Download
M src/pkg/image/draw/bench_test.go View 1 2 3 4 9 chunks +28 lines, -27 lines 0 comments Download
M src/pkg/image/draw/draw.go View 1 2 3 4 10 chunks +11 lines, -12 lines 0 comments Download
M src/pkg/image/draw/draw_test.go View 1 2 3 4 10 chunks +53 lines, -52 lines 0 comments Download
M src/pkg/image/gif/reader.go View 1 2 3 4 5 chunks +7 lines, -6 lines 0 comments Download
M src/pkg/image/image.go View 1 2 3 4 17 chunks +71 lines, -111 lines 0 comments Download
M src/pkg/image/image_test.go View 1 2 3 4 4 chunks +11 lines, -10 lines 0 comments Download
M src/pkg/image/jpeg/reader.go View 1 2 3 4 2 chunks +2 lines, -1 line 0 comments Download
M src/pkg/image/jpeg/writer_test.go View 1 2 3 4 2 chunks +2 lines, -1 line 0 comments Download
M src/pkg/image/names.go View 1 2 3 4 2 chunks +27 lines, -23 lines 0 comments Download
M src/pkg/image/png/reader.go View 1 2 3 4 10 chunks +26 lines, -25 lines 0 comments Download
M src/pkg/image/png/reader_test.go View 1 2 3 4 5 chunks +19 lines, -18 lines 0 comments Download
M src/pkg/image/png/writer.go View 1 2 3 4 8 chunks +12 lines, -11 lines 0 comments Download
M src/pkg/image/png/writer_test.go View 1 2 3 4 4 chunks +7 lines, -7 lines 0 comments Download
M src/pkg/image/tiff/reader.go View 1 2 3 4 7 chunks +10 lines, -9 lines 0 comments Download
M src/pkg/image/ycbcr/ycbcr.go View 1 2 3 4 5 chunks +5 lines, -4 lines 0 comments Download

Messages

Total messages: 7
r
http://codereview.appspot.com/5132048/diff/1001/src/pkg/color/color.go File src/pkg/color/color.go (right): http://codereview.appspot.com/5132048/diff/1001/src/pkg/color/color.go#newcode5 src/pkg/color/color.go:5: package color needs a package comment
12 years, 7 months ago (2011-09-28 16:12:47 UTC) #1
nigeltao
Hello r@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
12 years, 7 months ago (2011-09-30 03:23:07 UTC) #2
bsiegert
Looks very nice. Should the package name be "image/color"? --Benny.
12 years, 7 months ago (2011-09-30 06:57:58 UTC) #3
r
looks generally good but it should be in image/color. http://codereview.appspot.com/5132048/diff/9001/src/cmd/gofix/color.go File src/cmd/gofix/color.go (right): http://codereview.appspot.com/5132048/diff/9001/src/cmd/gofix/color.go#newcode22 src/cmd/gofix/color.go:22: ...
12 years, 7 months ago (2011-10-01 00:11:22 UTC) #4
nigeltao
PTAL.
12 years, 7 months ago (2011-10-02 09:54:46 UTC) #5
r
LGTM
12 years, 7 months ago (2011-10-03 20:25:07 UTC) #6
nigeltao
12 years, 7 months ago (2011-10-04 00:09:11 UTC) #7
*** Submitted as http://code.google.com/p/go/source/detail?r=f400d3afc555 ***

image: spin off a new color package out of the image package.

The spin-off renames some types. The new names are simply better:
image.Color              -> color.Color
image.ColorModel         -> color.Model
image.ColorModelFunc     -> color.ModelFunc
image.PalettedColorModel -> color.Palette
image.RGBAColor          -> color.RGBA
image.RGBAColorModel     -> color.RGBAModel
image.RGBA64Color        -> color.RGBA64
image.RGBA64ColorModel   -> color.RGBA64Model
(similarly for NRGBAColor, GrayColorModel, etc)

The image.ColorImage type stays in the image package, but is renamed:
image.ColorImage -> image.Uniform

The image.Image implementations (image.RGBA, image.RGBA64, image.NRGBA,
image.Alpha, etc) do not change their name, and gain a nice symmetry:
an image.RGBA is an image of color.RGBA, etc.

The image.Black, image.Opaque uniform images remain unchanged (although
their type is renamed from image.ColorImage to image.Uniform). The
corresponding color types (color.Black, color.Opaque, etc) are new.

Nothing in the image/ycbcr is renamed yet. The ycbcr.YCbCrColor and
ycbcr.YCbCrImage types will eventually migrate to color.YCbCr and
image.YCbCr, but that will be a separate CL.

R=r, bsiegert
CC=golang-dev
http://codereview.appspot.com/5132048
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b