Review of attachment 9063094 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me except for "poopulate" :) Didn't apply or test the patch yet. But I did test the last patch that I modified to be just like this and it worked fine.
Bug 1532388 Comment 163 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Review of attachment 9063094 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me except for "poopulate" :) Didn't apply or test the patch yet. But I did test the last patch that I modified to be just like this and it worked fine. Edit: At one point I noticed that smtp's SetClientId() wasn't called and I commented it out. It compiled OK but got a linker error "unknown identifier" so I put it back and the build finished. But maybe I did something wrong...
Review of attachment 9063094 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me except for "poopulate" :) Didn't apply or test the patch yet. But I did test the last patch that I modified to be just like this and it worked fine. Edit: At one point I noticed that smtp's SetClientId() wasn't called and I commented it out. It compiled OK but got a linker error "unknown identifier" so I put it back and the build finished. But maybe I did something wrong... Edit2 Re: comment 164: Dan, I should have read yours and Jorg's comments above closer. However, I would vote for not using readonly since, like you say, incoming always produces a setter and both may be needed in the future. Also, the compiler probably optimized out any function that is never actually called.