Closed Bug 1308651 Opened 3 years ago Closed 3 years ago

properties-db.js needs to be usefully diffable/patchable (i.e. it should not have 100,000-character lines)

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(firefox52 fixed)

RESOLVED FIXED
Firefox 52
Tracking Status
firefox52 --- fixed

People

(Reporter: dholbert, Assigned: gregtatum)

References

Details

Attachments

(1 file)

Right now, the (generated) source file properties-db.js has most of its contents on a line that is over 100,000 characters long (starting with exports.CSS_PROPERTIES)

It's got another line that's 15,000 characters long (starting with "+exports.PREFERENCES")

This means that whenever we need to regenerate this file (e.g. when we add/remove CSS properties), we just have to trust that the changes are sane, since the diff looks like:
-[around 100,000 characters]
+[around 100,000 characters]

This file needs to be generated in more bite-sized chunks (e.g. maybe a newline after every comma, or something like that), so that autogenerated changes to it can be verified / sanity-checked by a human.
In particular, neerja ran into a test-failure when changing some CSS properties over on bug 1300895 and had to run the mach command to regenerate the file. That produced a 240 KB diff, which no diff viewer can usefully display:
https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1300895&attachment=8799055

I'd like us to fix this file so that we can actually generate useful diffs before we patch it over in bug 1300895.

gtatum / tromey, based on hg log, it looks like you've been maintaining this file. Could one of you take this & adjust its autogeneration script to make it produce shorter lines?
(If it'd be possible to fix this in the next day or so, that would be awesome - this is blocking us from landing bug 1300895. 

[perhaps it doesn't technically *need* to block that bug -- but I think it should, because it doesn't make sense to me a patch to rename some CSS properties (bug 1300895's fix) should have to be 95% composed of a non-human-auditable autogenerated rewriting of a test data-file]
Flags: needinfo?(gtatum)
(by "next day or so" I meant business-day, of course - I know it's Friday right now. :))
Sounds totally reasonable. I originally had the generated code mixed in with non-generated code so I wanted to make it obvious. Now it's generated with a template and has warning messages at the top that it's auto-generated.
Flags: needinfo?(gtatum)
Comment on attachment 8799425 [details]
Bug 1308651 - Make css properties db diffable and human readable;

https://reviewboard.mozilla.org/r/84610/#review83162

Thank you.  This looks good.
Attachment #8799425 - Flags: review?(ttromey) → review+
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d59874236c6c
Make css properties db diffable and human readable; r=tromey
https://hg.mozilla.org/mozilla-central/rev/d59874236c6c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.