Skip to content

Commit 74e6638

Browse files
Remove timezone feature flag in languageservice (#341)
* Remove timezone feature flag in languageservice * Prettier * Address comment --------- Co-authored-by: Angel Kou <[email protected]>
1 parent f8b8b57 commit 74e6638

File tree

8 files changed

+15
-129
lines changed

8 files changed

+15
-129
lines changed

expressions/src/features.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ describe("FeatureFlags", () => {
5555
"missingInputsQuickfix",
5656
"blockScalarChompingWarning",
5757
"allowCaseFunction",
58-
"allowCronTimezone",
5958
"allowCopilotRequestsPermission"
6059
]);
6160
});

expressions/src/features.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,6 @@ export interface ExperimentalFeatures {
3535
*/
3636
allowCaseFunction?: boolean;
3737

38-
/**
39-
* Enable the timezone input in cron schedule mappings.
40-
* @default false
41-
*/
42-
allowCronTimezone?: boolean;
43-
4438
/**
4539
* Enable the copilot-requests permission in workflow permissions.
4640
* @default false
@@ -61,7 +55,6 @@ const allFeatureKeys: ExperimentalFeatureKey[] = [
6155
"missingInputsQuickfix",
6256
"blockScalarChompingWarning",
6357
"allowCaseFunction",
64-
"allowCronTimezone",
6558
"allowCopilotRequestsPermission"
6659
];
6760

languageservice/src/complete.test.ts

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -927,35 +927,7 @@ jobs:
927927
});
928928

929929
describe("schedule timezone completion", () => {
930-
it("includes timezone when allowCronTimezone is enabled", async () => {
931-
const input = `on:
932-
schedule:
933-
- |`;
934-
const result = await complete(...getPositionFromCursor(input), {
935-
featureFlags: new FeatureFlags({allowCronTimezone: true})
936-
});
937-
938-
expect(result).not.toBeUndefined();
939-
const labels = result.map(x => x.label);
940-
expect(labels).toContain("cron");
941-
expect(labels).toContain("timezone");
942-
});
943-
944-
it("excludes timezone when allowCronTimezone is disabled", async () => {
945-
const input = `on:
946-
schedule:
947-
- |`;
948-
const result = await complete(...getPositionFromCursor(input), {
949-
featureFlags: new FeatureFlags({allowCronTimezone: false})
950-
});
951-
952-
expect(result).not.toBeUndefined();
953-
const labels = result.map(x => x.label);
954-
expect(labels).toContain("cron");
955-
expect(labels).not.toContain("timezone");
956-
});
957-
958-
it("excludes timezone when no feature flags are provided", async () => {
930+
it("includes timezone for schedule", async () => {
959931
const input = `on:
960932
schedule:
961933
- |`;
@@ -964,7 +936,7 @@ describe("schedule timezone completion", () => {
964936
expect(result).not.toBeUndefined();
965937
const labels = result.map(x => x.label);
966938
expect(labels).toContain("cron");
967-
expect(labels).not.toContain("timezone");
939+
expect(labels).toContain("timezone");
968940
});
969941
});
970942

languageservice/src/complete.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,6 @@ export async function complete(
163163
values = filterActionRunsCompletions(values, path, parsedTemplate.value);
164164
}
165165

166-
// Filter `timezone` from schedule completions when the feature flag is disabled
167-
if (!config?.featureFlags?.isEnabled("allowCronTimezone") && parent?.definition?.key === "schedule") {
168-
values = values.filter(v => v.label !== "timezone");
169-
}
170-
171166
// Filter `copilot-requests` from permissions completions when the feature flag is disabled
172167
if (
173168
!config?.featureFlags?.isEnabled("allowCopilotRequestsPermission") &&

languageservice/src/validate.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,7 @@ async function validateWorkflow(textDocument: TextDocument, config?: ValidationC
8484
// Errors will be updated in the context. Attempt to do the conversion anyway in order to give the user more information
8585
const template = await getOrConvertWorkflowTemplate(result.context, result.value, textDocument.uri, config, {
8686
fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0,
87-
errorPolicy: ErrorPolicy.TryConversion,
88-
featureFlags: config?.featureFlags
87+
errorPolicy: ErrorPolicy.TryConversion
8988
});
9089

9190
// Validate expressions and value providers

workflow-parser/src/model/convert.test.ts

Lines changed: 6 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
/* eslint-disable @typescript-eslint/no-non-null-assertion */
2-
import {FeatureFlags} from "@actions/expressions/features";
32
import {nullTrace} from "../test-utils/null-trace.js";
43
import {parseWorkflow} from "../workflows/workflow-parser.js";
54
import {convertWorkflowTemplate, ErrorPolicy} from "./convert.js";
@@ -580,8 +579,8 @@ jobs:
580579
});
581580
});
582581

583-
describe("schedule timezone with feature flags", () => {
584-
it("allows timezone when allowCronTimezone is enabled", async () => {
582+
describe("schedule timezone", () => {
583+
it("allows timezone in schedule", async () => {
585584
const result = parseWorkflow(
586585
{
587586
name: "wf.yaml",
@@ -597,8 +596,7 @@ jobs:
597596
);
598597

599598
const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
600-
errorPolicy: ErrorPolicy.TryConversion,
601-
featureFlags: new FeatureFlags({allowCronTimezone: true})
599+
errorPolicy: ErrorPolicy.TryConversion
602600
});
603601

604602
expect(result.context.errors.getErrors()).toHaveLength(0);
@@ -609,57 +607,6 @@ jobs:
609607
});
610608
});
611609

612-
it("reports error when timezone is present but allowCronTimezone is disabled", async () => {
613-
const result = parseWorkflow(
614-
{
615-
name: "wf.yaml",
616-
content: `on:
617-
schedule:
618-
- cron: '0 0 * * *'
619-
timezone: America/New_York
620-
jobs:
621-
build:
622-
runs-on: ubuntu-latest`
623-
},
624-
nullTrace
625-
);
626-
627-
const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
628-
errorPolicy: ErrorPolicy.TryConversion,
629-
featureFlags: new FeatureFlags({allowCronTimezone: false})
630-
});
631-
632-
// When timezone feature is disabled, error points at the timezone key
633-
expect(result.context.errors.getErrors()).toHaveLength(1);
634-
expect(result.context.errors.getErrors()[0].message).toContain("Key 'timezone' is not supported");
635-
// Schedule entry is dropped due to unsupported key
636-
expect(template.events?.schedule).toHaveLength(0);
637-
});
638-
639-
it("reports error when timezone is present with no feature flags provided", async () => {
640-
const result = parseWorkflow(
641-
{
642-
name: "wf.yaml",
643-
content: `on:
644-
schedule:
645-
- cron: '0 0 * * *'
646-
timezone: America/New_York
647-
jobs:
648-
build:
649-
runs-on: ubuntu-latest`
650-
},
651-
nullTrace
652-
);
653-
654-
await convertWorkflowTemplate(result.context, result.value!, undefined, {
655-
errorPolicy: ErrorPolicy.TryConversion
656-
});
657-
658-
// Default is timezone disabled, so error points at the timezone key
659-
expect(result.context.errors.getErrors()).toHaveLength(1);
660-
expect(result.context.errors.getErrors()[0].message).toContain("Key 'timezone' is not supported");
661-
});
662-
663610
it("reports error when cron is missing from schedule entry", async () => {
664611
const result = parseWorkflow(
665612
{
@@ -675,8 +622,7 @@ jobs:
675622
);
676623

677624
const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
678-
errorPolicy: ErrorPolicy.TryConversion,
679-
featureFlags: new FeatureFlags({allowCronTimezone: true})
625+
errorPolicy: ErrorPolicy.TryConversion
680626
});
681627

682628
// Both schema validation and converter report the missing cron
@@ -689,7 +635,7 @@ jobs:
689635
expect(template.events?.schedule).toHaveLength(0);
690636
});
691637

692-
it("converts schedule without timezone when allowCronTimezone is enabled", async () => {
638+
it("converts schedule without timezone", async () => {
693639
const result = parseWorkflow(
694640
{
695641
name: "wf.yaml",
@@ -704,8 +650,7 @@ jobs:
704650
);
705651

706652
const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
707-
errorPolicy: ErrorPolicy.TryConversion,
708-
featureFlags: new FeatureFlags({allowCronTimezone: true})
653+
errorPolicy: ErrorPolicy.TryConversion
709654
});
710655

711656
expect(result.context.errors.getErrors()).toHaveLength(0);

workflow-parser/src/model/convert.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ export type WorkflowTemplateConverterOptions = {
4040
errorPolicy?: ErrorPolicy;
4141

4242
/**
43-
* Optional feature flags to control which experimental features are enabled.
43+
* Feature flags for experimental features.
44+
* This option is not currently used but keeping it for future use.
4445
*/
4546
featureFlags?: FeatureFlags;
4647
};
@@ -61,11 +62,6 @@ export async function convertWorkflowTemplate(
6162
const result = {} as WorkflowTemplate;
6263
const opts = getOptionsWithDefaults(options);
6364

64-
// Store feature flags in context state so converters can access them
65-
if (opts.featureFlags) {
66-
context.state["featureFlags"] = opts.featureFlags;
67-
}
68-
6965
if (context.errors.getErrors().length > 0 && opts.errorPolicy === ErrorPolicy.ReturnErrorsOnly) {
7066
result.errors = context.errors.getErrors().map(x => ({
7167
Message: x.message

workflow-parser/src/model/converter/events.ts

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import {FeatureFlags} from "@actions/expressions/features";
21
import {TemplateContext} from "../../templates/template-context.js";
32
import {MappingToken} from "../../templates/tokens/mapping-token.js";
43
import {SequenceToken} from "../../templates/tokens/sequence-token.js";
@@ -56,8 +55,7 @@ export function convertOn(context: TemplateContext, token: TemplateToken): Event
5655
// Schedule is the only event that can be a sequence, handle that separately
5756
if (eventName === "schedule") {
5857
const scheduleToken = item.value.assertSequence(`event ${eventName}`);
59-
const featureFlags = context.state["featureFlags"] as FeatureFlags | undefined;
60-
result.schedule = convertSchedule(context, scheduleToken, featureFlags);
58+
result.schedule = convertSchedule(context, scheduleToken);
6159
continue;
6260
}
6361

@@ -149,13 +147,7 @@ function convertFilter<T extends TypesFilterConfig & WorkflowFilterConfig & Vers
149147
return result;
150148
}
151149

152-
function convertSchedule(
153-
context: TemplateContext,
154-
token: SequenceToken,
155-
featureFlags?: FeatureFlags
156-
): ScheduleConfig[] | undefined {
157-
const flags = featureFlags ?? new FeatureFlags();
158-
const allowTimezone = flags.isEnabled("allowCronTimezone");
150+
function convertSchedule(context: TemplateContext, token: SequenceToken): ScheduleConfig[] | undefined {
159151
const result = [] as ScheduleConfig[];
160152

161153
for (const item of token) {
@@ -173,13 +165,8 @@ function convertSchedule(
173165
}
174166
config.cron = cron.value;
175167
} else if (key.value === "timezone") {
176-
if (allowTimezone) {
177-
const timezone = entry.value.assertString(`schedule timezone`);
178-
config.timezone = timezone.value;
179-
} else {
180-
context.error(key, `Key 'timezone' is not supported`);
181-
valid = false;
182-
}
168+
const timezone = entry.value.assertString(`schedule timezone`);
169+
config.timezone = timezone.value;
183170
} else {
184171
context.error(key, `Invalid schedule key`);
185172
valid = false;

0 commit comments

Comments
 (0)