Bug 68765 - Remove ENABLE(FTPDIR) and associated code
Summary: Remove ENABLE(FTPDIR) and associated code
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks: 68012
  Show dependency treegraph
 
Reported: 2011-09-24 13:46 PDT by Adam Barth
Modified: 2011-09-24 14:17 PDT (History)
1 user (show)

See Also:


Attachments
Patch (122.60 KB, patch)
2011-09-24 13:50 PDT, Adam Barth
mrowe: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2011-09-24 13:46:33 PDT
Remove ENABLE(FTPDIR) and associated code
Comment 1 Adam Barth 2011-09-24 13:50:14 PDT
Created attachment 108588 [details]
Patch
Comment 2 Mark Rowe (bdash) 2011-09-24 13:58:00 PDT
Why is this being removed given that it’s enabled and used by Safari on Windows?
Comment 3 Adam Barth 2011-09-24 14:01:59 PDT
> Why is this being removed given that it’s enabled and used by Safari on Windows?

Perhaps I misunderstood Platform.h.  This block of code makes it look like it is enabled only for PLATFORM(IOS):

http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/wtf/Platform.h#L572

Can you show me how to tell it is enabled on PLATFORM(WIN) so that I can avoid this sort of mistake in the future?
Comment 4 Mark Rowe (bdash) 2011-09-24 14:02:48 PDT
791	#if !defined(ENABLE_FTPDIR)
792	#define ENABLE_FTPDIR 1
793	#endif
Comment 5 Adam Barth 2011-09-24 14:04:04 PDT
Thanks.  To be clear, are you asking for this code to be keep or just asking why I've posted this patch to remove it?
Comment 6 Mark Rowe (bdash) 2011-09-24 14:05:07 PDT
Comment on attachment 108588 [details]
Patch

Please don’t remove code that other ports are actively using.
Comment 7 Mark Rowe (bdash) 2011-09-24 14:05:18 PDT
Is that clearer?
Comment 8 Adam Barth 2011-09-24 14:08:39 PDT
> Is that clearer?

Yes.  Thank you.

Please consider replying to this message on webkit-dev if there are other ENABLE macros slated for removal that you believe we should not remove:

http://www.mail-archive.com/webkit-dev@lists.webkit.org/msg16428.html

Alternatively, please feel encouraged to comment on this document, which contains updates based on the webkit-dev discussion:

https://docs.google.com/document/d/1MVbOTYkZxTj071-ZQI7Lsh7CdZJhS2_1OdU4bjYuaKA/edit?hl=en_US
Comment 9 Mark Rowe (bdash) 2011-09-24 14:10:29 PDT
Please consider double-checking your work before you propose deleting code that other people are using.
Comment 10 Adam Barth 2011-09-24 14:17:16 PDT
> Please consider double-checking your work before you propose deleting code that other people are using.

As part of this process, we are removing code that other folks are using.  For example, there were even folks using WML before we removed it.  That's why these changes are being discussed on webkit-dev and in these bug threads before they happen.

In any case, my goal here is not to make any controversial changes.  Thanks for providing feedback at this stage of the process.  The earlier in the process that you're able to provide feedback, the more efficient it is for the project as a whole.