722370 - Remove obsolete MOZ_WINSDK_TARGETVER checks in comm-central, after bug 721447

Status: VERIFIED FIXED

(MailNews Core :: Build Config, defect)

Description

[Serge Gautherie (:sgautherie)]

3 mail/ files and 1 suite/ one.

Comment 1

[Murali]

Hi, I'm interested in working on this bug. Could you please tell me more about what I'm supposed to do?

Comment 2

[David :Bienvenu]

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

Comment 3

[Murali]

Attachment: Patch 1

Could you please check whether this patch is somewhere in the region of okay? Thank you.

Comment 4

[David :Bienvenu]

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.

Comment 5

[Murali]

Attachment: Updated Patch

As per your comment, I've modified the patch. It now removes all checks against MOZ_NTDDI_LONGHORN. Please suggest necessary changes. Thank you.

Comment 6

[Serge Gautherie (:sgautherie)]

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.

Comment 7

[Murali]

Attachment: Updated Patch 2

Updated the patch, implementing the change you mentioned. Thanks a lot!

Comment 8

[Serge Gautherie (:sgautherie)]

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

[David :Bienvenu]

Comment on attachment 611313 [details] [diff] [review]
Updated Patch 2

thx for the patch!

Comment 10

[David :Bienvenu]

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!

Comment 11

[Murali]

Attachment: New Patch

Thanks a lot!

Comment 12

[Serge Gautherie (:sgautherie)]

Comment on attachment 611780 [details] [diff] [review]
New Patch

No more review is needed.
Include f= and r=, then add 'checkin-needed' keyword.

Comment 13

[Murali]

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

[Ludovic Hirlimann [:Usul]]

Comment on attachment 611780 [details] [diff] [review]
New Patch

Carrying feedback and review flags.

Comment 15

[Serge Gautherie (:sgautherie)]

(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

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Thanks for the patch!

http://hg.mozilla.org/comm-central/rev/31da82521973

Comment 17

[Serge Gautherie (:sgautherie)]

V.Fixed, per MXR search.