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

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=197973

--- Comment #18 from Ryosuke Niwa <rniwa at webkit.org> ---
(In reply to Zhifei Fang from comment #17)
> (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 agree that's valuable but the approach you took isn't right.

> 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.

That's precisely why we don't want the approach you took. We want the test data to be just JSON being sent by the server, and not something from which we construct such a reply. Because then we'd have to be concerned about the correctness of the test code to generate the reply. I have a number of experiences in which such a code was the cause of flaky tests, broken tests, etc...

> > > 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.

The guideline is not to use === or !== unless it's absolutely necessary. In general, I find that having to treat null, undefined, and 0 to be more of a annoyance. If we're writing code which relies on the difference between them, we have to re-examine the approach in the first place.

> > > 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.

Well, I've basically written this entire code base from scratch, and I never find that to be case.


> > > 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.
> 
> https://eslint.org/docs/rules/eqeqeq

That's precisely the kind of data I don't care about. I really don't care if someone with Ph.D. in computer sciences or someone coding in JS for 30 years would tell me what. The only thing I care about whether something makes sense for this particular code base, and I don't think it does.

-- 
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/e3bcf9a4/attachment.html>