[Webkit-unassigned] [Bug 197973] 'analysis-task-configurator-pane' does not update when switch from one analysis task to another

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 24 20:34:51 PDT 2019


--- Comment #17 from Zhifei Fang <zhifei_fang at apple.com> ---
(In reply to Ryosuke Niwa from comment #16)
> (In reply to Zhifei Fang from comment #15)
> > I agree to put the mock staff in other patch.
> We should still add tests but with the existing format.

I'd love to have something can test the whole page from fetched the network, this covers all scenario of this patch.

I think the existing format is more easier to be break, we directly create the data models from constructor or some method like constructXXXFromData, and need to make sure all the data models have been created in the right order, mock a fetch, and provide the json directly to the code will be more easier to maintain. It just mock the backend API and won't explicitly call any data model constructor. So if we modified the data model constructor or constructXXXFromData, the test will still work fine. 

> > My point of view is, the best practice is using === everywhere, since you
> > don't need to think about the type conversion, make sure the type of the
> > value, and always gives you an expected results.
> Given our existing coding convention is not to use == everywhere, I don't
> think we should do that. Consistency within our own codebase is more
> important than whatever common practice there is elsewhere.

I did a search, there are already 35 results in 13 files, and include something like event.key === "ArrowDown", xxx.length === 1, typeof(value) === 'number'; Please specify a guide line about when we should use ===, and when we should use ==. Do we just use if for compare null, undefined, do we use it when we are sure about the type are the same? I guess the easiest way and most correct way is we don't use ==, then there are no guide line need to be followed.

> > And for the same type variables, you have nothing to lose, then why not?
> Because I'd have to type one more character for no good reason. A dogmatic
> statement like "we should always do X because that's the best practice" is
> never helpful without a careful consideration as to whether a given practice
> is beneficial to the particular project and case we have at hand.

It will save you thinking time, because each time you use ==, you need to be very sure, that two variables you compare are the same type. 

> For example, a lot of people who are new to WebKit would claim that we
> should always wrap each statement in curly braces. Yet, in my 20+ years of
> writing C/C++ code without braces for single line statements, I've never
> once had a bug caused by missing curley braces.

No, this is different than code style, == will have type conversion, === will not, for most people, === produces the expected result. Single line have braces or not doesn't affect the running result.

> > This also make the code easier to maintain, think about what if the id() can return something special for special meaning?
> I disagree that always using === would make the code easier to maintain.
> Such an extraordinary claim requires an extraordinary justification, and I
> simply don't see it. And whether other people and/or other projects have
> observed or made such a claim is no basis to adopt the same approach in the
> perf dashboard because each project has its own unique set of requirements
> and challenges as well as a set of people working on it.


> > I suggest start to use === everywhere.
> As such, I'm opposed to this suggestion until such a time is reached that
> there is a conclusive empirical evidence is provided to support your
> hypothesis that it materially reduce the maintenance code of the codebase.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190625/66c62d56/attachment-0001.html>

More information about the webkit-unassigned mailing list