Bug 54066 - [NRWT] Remove encoding parameters in rebaseline.
Summary: [NRWT] Remove encoding parameters in rebaseline.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-08 22:15 PST by Hayato Ito
Modified: 2011-02-09 21:31 PST (History)
4 users (show)

See Also:


Attachments
remove-encoding-parameters (4.54 KB, patch)
2011-02-08 22:28 PST, Hayato Ito
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hayato Ito 2011-02-08 22:15:09 PST
This is a follow up patch, after this comment,
https://bugs.webkit.org/show_bug.cgi?id=53071#c6

We can remove encoding parameters because we can assume all rebaseline data can be written to files in binary mode.
Comment 1 Hayato Ito 2011-02-08 22:28:14 PST
Created attachment 81752 [details]
remove-encoding-parameters
Comment 2 Tony Chang 2011-02-09 10:43:30 PST
Comment on attachment 81752 [details]
remove-encoding-parameters

Thanks!
Comment 3 Dirk Pranke 2011-02-09 13:22:46 PST
As long as nothing breaks, I guess this change is fine, but I still feel kind of uncomfortable about it. Perhaps we should also change the expected_text() and expected_checksum() methods in port/base.py to use read_binary_file() instead of read_text_file()?
Comment 4 Tony Chang 2011-02-09 13:42:48 PST
(In reply to comment #3)
> As long as nothing breaks, I guess this change is fine, but I still feel kind of uncomfortable about it. Perhaps we should also change the expected_text() and expected_checksum() methods in port/base.py to use read_binary_file() instead of read_text_file()?

expected_text() already uses read_binary_file().  expected_checksum() could use either since it's just reading ascii.  It's probably a bit safer to use read_text_file(), but I don't feel strongly one way or another.
Comment 5 Dirk Pranke 2011-02-09 13:49:29 PST
(In reply to comment #4)
> (In reply to comment #3)
> > As long as nothing breaks, I guess this change is fine, but I still feel kind of uncomfortable about it. Perhaps we should also change the expected_text() and expected_checksum() methods in port/base.py to use read_binary_file() instead of read_text_file()?
> 
> expected_text() already uses read_binary_file().  expected_checksum() could use either since it's just reading ascii.  It's probably a bit safer to use read_text_file(), but I don't feel strongly one way or another.

You're right. Apparently I can't read. Anyway, I would feel better if we changed expected_checksum() to use read_binary_file() as well, for consistency.
Comment 6 Hayato Ito 2011-02-09 21:29:14 PST
Thank you for the reviews. I'll commit the patch after I change expected_checksum() to use read_binary_file().

(In reply to comment #5)
> You're right. Apparently I can't read. Anyway, I would feel better if we changed expected_checksum() to use read_binary_file() as well, for consistency.
Comment 7 Hayato Ito 2011-02-09 21:31:48 PST
Committed r78175: <http://trac.webkit.org/changeset/78175>