Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use 'compatibility flag' language consistently #763

Merged
merged 1 commit into from
Jun 19, 2023

Conversation

irvinebroque
Copy link
Collaborator

refs EW-7514

  • Previously we used the language "feature flag" and "compatibility flag" interchangeably
  • This was ambiguous, since on Cloudflare and other platforms, "feature flag" refers to a separate concept and system, that requires internal action to enable.
  • This PR updates all references to "feature flag" to use the "compatibility flag" language, to ensure that when people encounter error messages, it is clear that these are in reference to compatibility flags, as described here in the Workers docs.

@irvinebroque irvinebroque marked this pull request as ready for review June 12, 2023 23:34
Copy link
Contributor

@ohodson ohodson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Brendan

When looking at this PR and existing code, there's also features.h (compatibility-flags.h?) and the FeatureFlags therein that should probably be aligned with this, but this is a cross repo change that doesn't need to be part of this change.

@@ -485,7 +485,8 @@ jsg::Ref<ReadableStream> ReadableStream::constructor(
JSG_REQUIRE(FeatureFlags::get(js).getStreamsJavaScriptControllers(),
Error,
"To use the new ReadableStream() constructor, enable the "
"streams_enable_constructors feature flag.");
"streams_enable_constructors compatibility flag. "
"Refer to the docs for more information: https://developers.cloudflare.com/workers/platform/compatibility-dates/#compatibility-flags");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text here appears 3 times so could go into a convenience macro. Perhaps in features.h along the lines of:

#define COMPATIBILITY_NOTICE(Thing, FlagForThing)                                 \
  "To use the new " Thing " enable the " FlagForThing " compatibility flag."      \
  "Refer to the docs for more information: "                                      \
  "https://developers.cloudflare.com/workers/platform/compatibility-dates/#compatibility-flags"

@ohodson ohodson requested review from dom96 and jasnell June 13, 2023 08:38
@jasnell
Copy link
Member

jasnell commented Jun 14, 2023

Yeah, I went with FeatureFlags for the new thing because of the existing language that was already there. I'd made a note to revisit it but it was a low priority.

@irvinebroque irvinebroque merged commit 6134d02 into cloudflare:main Jun 19, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants