1307905 - Ignore irrelevant properties in stub generation

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P2)

Description

[Lin Clark [:linclark]]

Originally posted by:linclark

see https://github.com/devtools-html/gecko-dev/issues/257

When stubs are generated, the packet stubs will always have a different timestamp. For properties like these, we should make sure the incoming packet has the expected structure and then overwrite it with the existing stub's value so they don't have to change with each PR that adds a stub.

- [ ] timestamp
- [ ] actor ID

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: bug_1307905.patch

Lin, I got this working a first time, by going manually through the different properties and type of packets, but I thought it was not ideal.
So I changed things in order to declare the irrelevant properties in a selector-like format.
I'm not a hundred percent convinced by this over the first method I did, so I wonder what you think of this.

Comment 2

[Lin Clark [:linclark]]

This generalized code seems kind of complex. I'm not sure what the more manual approach would look like, but I'm guessing it might be clearer. I'd probably lean towards the manual approach since we probably won't have to make too many additions, but I leave it up to your judgement.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1307905 - Ignore irrelevant properties in stub generation;

This can be tested by inserting new stubs at first position, in order to change
the actor id on the existing stubs, and then check in the resulting diff that
actor id, timestamp and timer properties are not modified.

Review commit: https://reviewboard.mozilla.org/r/89530/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89530/

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8805919 [details]
Bug 1307905 - Ignore irrelevant properties in stub generation;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89530/diff/1-2/

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

> This generalized code seems kind of complex. I'm not sure what the more manual approach would look like, but I'm guessing it might be clearer. I'd probably lean towards the manual approach since we probably won't have to make too many additions, but I leave it up to your judgement.

I agree with you. I revert back to the manual approach

Comment 6

[Lin Clark [:linclark]]

Comment on attachment 8805919 [details]
Bug 1307905 - Ignore irrelevant properties in stub generation;

https://reviewboard.mozilla.org/r/89530/#review91230

I haven't tested it, but this looks good to me. The only thing I'd suggest is adding a comment before each block to describe what's getting cleaned. Thanks!

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8805919 [details]
Bug 1307905 - Ignore irrelevant properties in stub generation;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89530/diff/2-3/

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

Thanks for the review Lin ! I wanted bgrins to take it but he's not accepting review (probably hasn't switch back on when he came back), and I didn't saw him on IRC since Friday to tell him :)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/342db7014f26
Ignore irrelevant properties in stub generation; r=linclark

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/342db7014f26