Hi Xan, FYI, if you’re writing JSC stress tests, you can add test specific options by putting the following at the top of the test file:
//@ requireOptions(“--myOption=true”, “--myOtherOption=1234”) For LayoutTests, you can add the following to the top line of the test html file: <!-- webkit-test-runner [ jscOptions=--myOptions=true ] --> This way, the feature can be disabled by default but still receive testing. Mark > On Feb 14, 2019, at 9:27 AM, Maciej Stachowiak <[email protected]> wrote: > > > >> On Feb 14, 2019, at 1:37 AM, Xan Lopez <[email protected] >> <mailto:[email protected]>> wrote: >> >> Hi Maciej, >> >> the first patches had the flag indeed, so it should be easy to add it back >> to the patch. Not sure what's the usual procedure, but I guess it makes >> sense to enable it by default in the bug so that the bots keep testing the >> code? Then we'll disable it before landing if that's our decision. > > If it’s a runtime flag then it’s possible to have the tests run without > having it enabled by default. This is one reason runtime flags are the > preferred choice, the feature can be running tests all along while still > getting further polish and refinement. > > For a compile-time flag, your approach sounds reasonable to me (patch that > includes a default-on flag, ideally with a note in the ChangeLog that it will > be flipped before landing). > > We almost never turn on features by default right in the patch where they > first land, unless it is really small and simple, to reduce risk of anyone > accidentally shipping it if they happen to cut a release branch soon after it > lands (and before it has had enough bake time). > > BTW I appreciate the contribution of such a substantial feature, I regret > that you haven’t gotten more active review, and I am glad that Saam is now > giving you another review pass. > > Cheers, > Maciej > >> >> Cheers, >> Xan >> >> On Thu, Feb 14, 2019 at 8:47 AM Maciej Stachowiak <[email protected] >> <mailto:[email protected]>> wrote: >> >> I left the boring review feedback that this work should be behind a feature >> flag. Mentioning it here because this may apply to other feature patches you >> have in progress. (I am not qualified to review the substance of what the >> patch is doing.) >> >> > On Feb 13, 2019, at 1:51 PM, [email protected] <mailto:[email protected]> >> > wrote: >> > >> > Hi WebKitters, >> > >> > My colleagues at Igalia have been working on a number of JS language >> > features! We want WebKit to have implementations in order to provide >> > feedback for TC39, and to help meet the requirements to have them merged >> > into the specification proper. >> > >> > Unfortunately, JSC suggested reviewers have been occupied for the past 6 >> > months, unable to provide feedback and help us upstream the patches. I'm >> > reaching out to see if there are other folks reading webkit-dev who might >> > be able to check on the work, see how it looks, and help us get this stuff >> > upstream. We've been doing internal reviews, but unfortunately don't yet >> > have any JSC reviewers on our team. >> > >> > You can find the patch for instance class fields at >> > https://bugs.webkit.org/show_bug.cgi?id=174212 >> > <https://bugs.webkit.org/show_bug.cgi?id=174212>. Any kind of constructive >> > feedback (from anyone) would be much appreciated :) >> > >> > _______________________________________________ >> > webkit-dev mailing list >> > [email protected] <mailto:[email protected]> >> > https://lists.webkit.org/mailman/listinfo/webkit-dev >> > <https://lists.webkit.org/mailman/listinfo/webkit-dev> >> >> _______________________________________________ >> webkit-dev mailing list >> [email protected] <mailto:[email protected]> >> https://lists.webkit.org/mailman/listinfo/webkit-dev >> <https://lists.webkit.org/mailman/listinfo/webkit-dev> > > _______________________________________________ > webkit-dev mailing list > [email protected] > https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________ webkit-dev mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-dev
