RESOLVED FIXED 191933
[ews-app] Add a config file
https://bugs.webkit.org/show_bug.cgi?id=191933
Summary [ews-app] Add a config file
Aakash Jain
Reported 2018-11-23 17:51:46 PST
We should add a config file to store all the configuration in one place.
Attachments
Proposed patch (3.01 KB, patch)
2018-11-23 19:04 PST, Aakash Jain
lforschler: review+
Aakash Jain
Comment 1 2018-11-23 19:04:24 PST
Created attachment 355549 [details] Proposed patch Part of patch series. Therefore wouldn't apply to ToT without applying other patches first.
Kocsen Chung
Comment 2 2018-11-26 14:56:10 PST
Comment on attachment 355549 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=355549&action=review > Tools/BuildSlaveSupport/ews-app/ews/config.py:1 > +# Copyright (C) 2018 Apple Inc. All rights reserved. We should be able to just leverage the Django Settings file: https://docs.djangoproject.com/en/2.1/topics/settings/ Located in: Tools/BuildSlaveSupport/ews-app/settings.py
Aakash Jain
Comment 3 2018-11-26 19:15:27 PST
Radar WebKit Bug Importer
Comment 4 2018-11-26 19:16:26 PST
Aakash Jain
Comment 5 2018-11-27 05:33:18 PST
> We should be able to just leverage the Django Settings file: We could and I did considered that, but it doesn't look like a clean approach to me. I want to keep the app settings and project settings separate. There are a large number of project settings in Django settings file. Adding this app's configuration there might make it unreadable and hard to find.
Kocsen Chung
Comment 6 2018-11-27 11:35:59 PST
> Adding this app's configuration there might make it unreadable and hard to find. I disagree - since Django developers would _expect_ to find these variables in the settings.py file to begin with.
Note You need to log in before you can comment on or make changes to this bug.