Closed
Bug 1135131
Opened 10 years ago
Closed 10 years ago
remove support for json file format from talos configuration.py
Categories
(Testing :: Talos, defect)
Testing
Talos
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jmaher, Assigned: ujjwal)
References
Details
Attachments
(1 file, 1 obsolete file)
9.05 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
right now configuration.py has support for json and yaml, we only use yaml, so lets make that our default.
We could effectively remove all abstraction code around the reading/writing of the specific format as well as detecting filenames and types.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8571407 -
Flags: review?(jmaher)
Reporter | ||
Comment 2•10 years ago
|
||
Comment on attachment 8571407 [details] [diff] [review]
bug-1135131-remove-json.diff
Review of attachment 8571407 [details] [diff] [review]:
-----------------------------------------------------------------
I really like this patch- lets push a bit harder and see if we can remove even more lines of code as recommended.
::: talos/configuration.py
@@ +51,2 @@
>
> if yaml:
could we remove this if yaml line?
@@ +470,2 @@
>
> + provider = self.configuration_provider_from_format(format)
we could get rid of the filename2format, and this configuration_provider_from_format. Both the functions and the lines here in serialize could be removed since we have just yml and can assume the sole type. No need to adjust the extensions either.
@@ +486,4 @@
>
> # deserialize the data
> + provider = self.configuration_provider_from_format(format)
> + if not provider:
I think this stuff could be simplified knowing we only have .yml support!
Attachment #8571407 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8571407 -
Attachment is obsolete: true
Attachment #8571472 -
Flags: review?(jmaher)
Reporter | ||
Comment 4•10 years ago
|
||
Comment on attachment 8571472 [details] [diff] [review]
bug-1135131-remove-json.diff
Review of attachment 8571472 [details] [diff] [review]:
-----------------------------------------------------------------
this looks great!
Attachment #8571472 -
Flags: review?(jmaher) → review+
Reporter | ||
Comment 5•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → w.ujjwal
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•