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 <m...@apple.com> wrote:
> 
> 
> 
>> On Feb 14, 2019, at 1:37 AM, Xan Lopez <x...@gnome.org 
>> <mailto:x...@gnome.org>> 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 <m...@apple.com 
>> <mailto:m...@apple.com>> 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, ca...@igalia.com <mailto:ca...@igalia.com> 
>> > 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
>> > webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>> > https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> > <https://lists.webkit.org/mailman/listinfo/webkit-dev>
>> 
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org>
>> https://lists.webkit.org/mailman/listinfo/webkit-dev 
>> <https://lists.webkit.org/mailman/listinfo/webkit-dev>
> 
> _______________________________________________
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Reply via email to