Closed
Bug 722370
Opened 12 years ago
Closed 12 years ago
Remove obsolete MOZ_WINSDK_TARGETVER checks in comm-central, after bug 721447
Categories
(MailNews Core :: Build Config, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
Thunderbird 14.0
People
(Reporter: sgautherie, Assigned: murali.sr92)
References
()
Details
Attachments
(1 file, 3 obsolete files)
8.41 KB,
patch
|
Usul
:
review+
Usul
:
feedback+
|
Details | Diff | Splinter Review |
3 mail/ files and 1 suite/ one.
Reporter | ||
Updated•12 years ago
|
Summary: Remove obsolete MOZ_WINSDK_TARGETVER checks in comm-central, after bug 699385 → Remove obsolete MOZ_WINSDK_TARGETVER checks in comm-central, after bug 721447
Reporter | ||
Updated•12 years ago
|
Hi, I'm interested in working on this bug. Could you please tell me more about what I'm supposed to do?
Comment 2•12 years ago
|
||
first thing is to use MXR to find them: http://mxr.mozilla.org/comm-central/search?string=MOZ_WINSDK_TARGETVER and then figure out which ones we don't need because we no longer support the versions of Windows they're checking for. (I don't think we need any of the checks, fwiw). Then, propose a patch that removes them and attach it here, and ask for a code review. I'm not sure how much you've done with Thunderbird, but here are build instructions - https://developer.mozilla.org/en/Simple_Thunderbird_build
Reporter | ||
Updated•12 years ago
|
Whiteboard: [good first bug] → [good first bug][mentor=dbienvenu][lang=c++]
Could you please check whether this patch is somewhere in the region of okay? Thank you.
Attachment #611266 -
Flags: feedback?(dbienvenu)
Comment 4•12 years ago
|
||
Comment on attachment 611266 [details] [diff] [review] Patch 1 thx for the patch. You should only remove the checks against MOZ_NTDDI_LONGHORN, not all the checks. So the changes that remove that check are good, but I think the more generic removals are wrong.
Attachment #611266 -
Flags: feedback?(dbienvenu) → feedback-
As per your comment, I've modified the patch. It now removes all checks against MOZ_NTDDI_LONGHORN. Please suggest necessary changes. Thank you.
Attachment #611266 -
Attachment is obsolete: true
Attachment #611276 -
Flags: review?(dbienvenu)
Reporter | ||
Comment 6•12 years ago
|
||
Comment on attachment 611276 [details] [diff] [review] Updated Patch Review of attachment 611276 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +788,5 @@ > fi > > AC_DEFINE_UNQUOTED(MOZ_WINSDK_TARGETVER,0x$MOZ_WINSDK_TARGETVER) > # Definitions matching sdkddkver.h > AC_DEFINE_UNQUOTED(MOZ_NTDDI_WS03, 0x05020000) Remove the MOZ_NTDDI_WS03 line too.
Attachment #611276 -
Flags: feedback+
Updated the patch, implementing the change you mentioned. Thanks a lot!
Attachment #611276 -
Attachment is obsolete: true
Attachment #611313 -
Flags: feedback?(sgautherie.bz)
Attachment #611276 -
Flags: review?(dbienvenu)
Reporter | ||
Updated•12 years ago
|
Attachment #611313 -
Flags: review?(dbienvenu)
Attachment #611313 -
Flags: feedback?(sgautherie.bz)
Attachment #611313 -
Flags: feedback+
Reporter | ||
Comment 8•12 years ago
|
||
See https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Comment 9•12 years ago
|
||
Comment on attachment 611313 [details] [diff] [review] Updated Patch 2 thx for the patch!
Attachment #611313 -
Flags: review?(dbienvenu) → review+
Comment 10•12 years ago
|
||
Murali, if you can look at the link Serge helpfully posted about how to generate a patch for somebody else to checkin, follow those instructions, attach a new patch, and add the checkin-needed keyword, we can land your patch. Thx again!
Assignee | ||
Comment 11•12 years ago
|
||
Thanks a lot!
Attachment #611313 -
Attachment is obsolete: true
Attachment #611780 -
Flags: review?
Reporter | ||
Comment 12•12 years ago
|
||
Comment on attachment 611780 [details] [diff] [review] New Patch No more review is needed. Include f= and r=, then add 'checkin-needed' keyword.
Attachment #611780 -
Flags: review?
Assignee | ||
Comment 13•12 years ago
|
||
Hi, I am unable to edit the keywords (I think I need to be assigned the bug first?). Also, are there any parameters for f and r? Thanks!
Comment 14•12 years ago
|
||
Comment on attachment 611780 [details] [diff] [review] New Patch Carrying feedback and review flags.
Attachment #611780 -
Flags: review+
Attachment #611780 -
Flags: feedback+
Updated•12 years ago
|
Reporter | ||
Comment 15•12 years ago
|
||
(In reply to Murali from comment #13) > I am unable to edit the keywords (I think I need to be assigned the bug > first?). Oh. Ludovic did it... > Also, are there any parameters for f and r? f=sgautherie r=dbienvenu
Reporter | ||
Updated•12 years ago
|
Flags: in-testsuite-
Hardware: x86 → All
Whiteboard: [good first bug][mentor=dbienvenu][lang=c++]
Target Milestone: --- → Thunderbird 14.0
Comment 16•12 years ago
|
||
Thanks for the patch! http://hg.mozilla.org/comm-central/rev/31da82521973
You need to log in
before you can comment on or make changes to this bug.
Description
•