Bug 191933 - [ews-app] Add a config file
Summary: [ews-app] Add a config file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-23 17:51 PST by Aakash Jain
Modified: 2018-11-27 11:35 PST (History)
5 users (show)

See Also:


Attachments
Proposed patch (3.01 KB, patch)
2018-11-23 19:04 PST, Aakash Jain
lforschler: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2018-11-23 17:51:46 PST
We should add a config file to store all the configuration in one place.
Comment 1 Aakash Jain 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.
Comment 2 Kocsen Chung 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
Comment 3 Aakash Jain 2018-11-26 19:15:27 PST
Committed r238530: <http://trac.webkit.org/changeset/238530>
Comment 4 Radar WebKit Bug Importer 2018-11-26 19:16:26 PST
<rdar://problem/46264850>
Comment 5 Aakash Jain 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.
Comment 6 Kocsen Chung 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.