Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat(publish): Use craft config from merge target if configured #3451

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ jobs:
with:
path: __repo__
repository: getsentry/${{ fromJSON(steps.inputs.outputs.result).repo }}
# If we extracted `merge_target` from the parsed issue, check out the merge target branch, otherwise fall back to the default branch
ref: ${{ fromJSON(steps.inputs.outputs.result).craft_config_branch == 'merge_target' && fromJSON(steps.inputs.outputs.result).merge_target || '' }}
token: ${{ secrets.GH_SENTRY_BOT_PAT }}
fetch-depth: 0

Expand Down
109 changes: 72 additions & 37 deletions src/modules/__tests__/details-from-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,34 +35,32 @@ Assign the **accepted** label to this issue to approve the release.
test("parse inputs", async () => {
const result = await detailsFromContext(inputsArgs);
expect(result).toEqual({
dry_run: "",
merge_target: "custom-branch",
path: ".",
repo: "sentry",
requester: "BYK",
targets: [
"github",
"docker[latest]",
],
version: "21.3.1",
});
dry_run: "",
merge_target: "custom-branch",
craft_config_branch: "default",
path: ".",
repo: "sentry",
requester: "BYK",
targets: ["github", "docker[latest]"],
version: "21.3.1",
});
});

test("can parse version containing +", async () => {
const result = await detailsFromContext({
context: {
repo: { owner: "getsentry", repo: "publish" },
payload: {
issue: {
number: "123",
title: "publish: getsentry/[email protected]+sentry1",
body: "Requested by: @example",
labels: [],
}
}
}
});
expect(result.version).toEqual('4.2.6+sentry1');
const result = await detailsFromContext({
context: {
repo: { owner: "getsentry", repo: "publish" },
payload: {
issue: {
number: "123",
title: "publish: getsentry/[email protected]+sentry1",
body: "Requested by: @example",
labels: [],
},
},
},
});
expect(result.version).toEqual("4.2.6+sentry1");
});

const defaultTargetInputsArgs = {
Expand Down Expand Up @@ -94,20 +92,57 @@ Assign the **accepted** label to this issue to approve the release.
test("Do not extract merge_target value if its a default value", async () => {
const result = await detailsFromContext(defaultTargetInputsArgs);
expect(result).toEqual({
dry_run: "",
merge_target: "",
path: ".",
repo: "sentry",
requester: "BYK",
targets: [
"github",
"docker[latest]",
],
version: "21.3.1",
});
dry_run: "",
craft_config_branch: "default",
merge_target: "",
path: ".",
repo: "sentry",
requester: "BYK",
targets: ["github", "docker[latest]"],
version: "21.3.1",
});
});

test("throw error when context is missing the issue payload", async () => {
const fn = () => detailsFromContext({ context: {} });
expect(fn).rejects.toThrow('Issue context is not defined');
expect(fn).rejects.toThrow("Issue context is not defined");
});

test("Extracts the craft config branch from the issue body", async () => {
const issueWithCraftConfigBranch = {
context: {
repo: { owner: "getsentry", repo: "publish" },
payload: {
issue: {
number: "223",
title: "publish: getsentry/[email protected]",
body: `
Requested by: @Lms24
Merge target: main
Using Craft config from: merge_target
Quick links:
* [View changes](https://github.com/getsentry/sentry-migr8/compare/438d5013dd3fa977a42c313ea12f8e1bc88a23fc...refs/heads/main)
* [View check runs](https://github.com/getsentry/sentry-migr8/commit/18c0fb1bd2afa60408cbf32627a09fd636a9ad42/checks/)
Assign the **accepted** label to this issue to approve the release. Leave a comment containing \`#retract\` under this issue to retract the release (original issuer only).
### Targets\r
- [x] npm\r
- [ ] github\r
`,
labels: ["accepted"],
},
},
},
};

const result = await detailsFromContext(issueWithCraftConfigBranch);
expect(result).toEqual({
dry_run: "",
merge_target: "main",
craft_config_branch: "merge_target",
path: ".",
repo: "sentry",
requester: "Lms24",
targets: ["npm"],
version: "21.3.1",
});
});
28 changes: 20 additions & 8 deletions src/modules/details-from-context.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
async function detailsFromContext({ context }) {
if (!context || !context.payload || !context.payload.issue) {
throw new Error('Issue context is not defined');
throw new Error("Issue context is not defined");
Copy link
Member Author

Choose a reason for hiding this comment

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

formatting change

}

const titleParser = /^publish: (?:getsentry\/)?(?<repo>[^/@]+)(?<path>\/[\w./-]+)?@(?<version>[\w.+-]+)$/;
const titleParser =
/^publish: (?:getsentry\/)?(?<repo>[^/@]+)(?<path>\/[\w./-]+)?@(?<version>[\w.+-]+)$/;
Comment on lines +6 to +7
Copy link
Member Author

Choose a reason for hiding this comment

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

formatting change

Copy link
Member

Choose a reason for hiding this comment

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

please discard the unrelated formatting changes from your PR so I can review the actual change

const titleMatch = context.payload.issue.title.match(titleParser).groups;
const dry_run = context.payload.issue.labels.some((l) => l.name === "dry-run")
? "1"
Expand All @@ -14,10 +15,10 @@ async function detailsFromContext({ context }) {
// - Cannot have multiple consecutive hyphens.
// - Cannot begin or end with a hyphen.
// - Maximum 39 characters.
const requesterParser = /^Requested by: @(?<requester>[a-zA-Z\d](?:[a-zA-Z\d]|-(?=[a-zA-Z\d])){0,38})$/m;
const { requester } = context.payload.issue.body.match(
requesterParser
).groups;
const requesterParser =
Copy link
Member Author

Choose a reason for hiding this comment

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

formatting change

/^Requested by: @(?<requester>[a-zA-Z\d](?:[a-zA-Z\d]|-(?=[a-zA-Z\d])){0,38})$/m;
const { requester } =
context.payload.issue.body.match(requesterParser).groups;

// https://docs.github.com/en/get-started/using-git/dealing-with-special-characters-in-branch-and-tag-names#naming-branches-and-tags
const mergeTargetParser = /^Merge target: (?<merge_target>[\w.\-/]+)$/m;
Expand All @@ -27,7 +28,17 @@ async function detailsFromContext({ context }) {
merge_target = mergeTargetMatch.groups.merge_target || "";
}

const targetsParser = /^(?!### Targets$\s)(?: *- \[[ x]\] [\w.[\]-]+$(?:\r?\n)?)+/m;
const craftConfigParser =
/^Using Craft config from: (?<config_branch>[\w._/]+)$/m;
const craftConfigMatch = context.payload.issue.body.match(craftConfigParser);
let craft_config_branch =
(craftConfigMatch &&
craftConfigMatch.groups &&
craftConfigMatch.groups.config_branch) ||
"default";

const targetsParser =
/^(?!### Targets$\s)(?: *- \[[ x]\] [\w.[\]-]+$(?:\r?\n)?)+/m;
const targetsMatch = context.payload.issue.body.match(targetsParser);
let targets;
if (targetsMatch) {
Expand All @@ -41,10 +52,11 @@ async function detailsFromContext({ context }) {
...titleMatch,
dry_run,
merge_target,
craft_config_branch,
path,
requester,
targets,
};
};
}

module.exports = detailsFromContext;
Loading