[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 #21 from Zhifei Fang <zhifei_fang at apple.com> ---
(In reply to Ryosuke Niwa from comment #18)
> (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...
> 

Ok, I can also make a fake server directly, use the existing PHP code to server some fake data, we can always fake something directly from server side. If we directly use constructors for integration test, it can be easily a false pass test. My point is we should cover the whole page logic instead of fake data model by data model and put them together. Because we still need to worry about the correctness of this part as same as you mentioned.

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

That's typically not enough, if you really want to default to use == and use === when necessary.

something == false/true 
// "0" == false => true
// "1" == false => false
// "0" == true => false
// "1" == true => true 
// etc

in most case, I'd love to see this has no type conversion

something == ''
// 0 == '' => true 

Also, it will support something like
// '1' == 1 => true
to use == in such case encourage people use it to compare things without explicitly convert the type, in my many experience, those can cause bugs very easily.

My point is, if your initial thought about don't type an extra = for saving time, you probably will cost more time to think about the type, in most case, I cannot see the benefits.

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

I think you should treat those case by case, take them all is equal to reject them all.

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