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.

Sign up for GitHub

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

Jump to bottom

feat: Add ability to define PR title #125

Merged
merged 3 commits into from
May 9, 2019
Merged

feat: Add ability to define PR title #125

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0864f16
Add ability to define pr title
cjskillingstad May 7, 2019
fe109c0
Remove eval and use replace instead
cjskillingstad May 8, 2019
8e9bbe5
Remove dollar signs from pr title string
cjskillingstad May 9, 2019
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
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ The above commands will start an interactive prompt. You can use the `arrow keys
| --labels | Pull request labels | string |
| --multiple | Backport multiple commits and/or branches | boolean |
| --prDescription | Description to be added to pull request | string |
| --prTitle | Title for the pull request | string |
| --sha | Commit sha to backport | string |
| --upstream | Name of repository | string |
| --username | Github username | string |
Expand Down
11 changes: 11 additions & 0 deletions docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,17 @@ Example: `["backport", "apm-team"]`

CLI: `--labels myLabel --labels myOtherLabel`

#### `prTitle`

Text that will be the pull request title. Note: You can pass a template literal string to access the base branch (`baseBranch`) and commit message (`commitMessages`) strings.
If there are multiple commits the commit messages will be concatenated and separated by pipes.

Example: `"${commitMessages} backport for ${baseBranch}"`

Default: `"[${baseBranch}] ${commitMessages}"`

CLI: `--prTitle "My PR Title"`

#### `prDescription`

Text that will be added to the pull request body.
Expand Down
6 changes: 6 additions & 0 deletions src/options/cliArgs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ export function getOptionsFromCliArgs(
description: 'Backport to multiple branches',
type: 'boolean'
})
.option('prTitle', {
default: configOptions.prTitle,
description: 'Title of pull request',
type: 'string'
})
.option('prDescription', {
default: configOptions.prDescription,
description: 'Description to be added to pull request',
Expand Down Expand Up @@ -79,6 +84,7 @@ export function getOptionsFromCliArgs(
multiple: cliArgs.multiple,
multipleBranches: cliArgs.multipleBranches || cliArgs.multiple,
multipleCommits: cliArgs.multipleCommits || cliArgs.multiple,
prTitle: cliArgs.prTitle,
prDescription: cliArgs.prDescription,
sha: cliArgs.sha,
upstream: cliArgs.upstream,
Expand Down
1 change: 1 addition & 0 deletions src/options/config/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export async function getOptionsFromConfigFiles() {
multipleBranches: true,
all: false,
labels: [] as string[],
prTitle: '[${baseBranch}] ${commitMessages}',

// options from config files
...globalConfig,
Expand Down
1 change: 1 addition & 0 deletions src/options/config/globalConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { getGlobalConfigPath, getReposPath } from '../../services/env';
interface GlobalConfig {
username?: string;
accessToken?: string;
prTitle?: string;
prDescription?: string;

// the following are overwritable by project config:
Expand Down
2 changes: 2 additions & 0 deletions src/options/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export function validateOptions({
multiple,
multipleBranches,
multipleCommits,
prTitle,
prDescription,
sha,
upstream,
Expand Down Expand Up @@ -84,6 +85,7 @@ export function validateOptions({
multiple,
multipleBranches,
multipleCommits,
prTitle,
prDescription,
sha,
upstream,
Expand Down
19 changes: 15 additions & 4 deletions src/steps/doBackportVersions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export function doBackportVersions(
branches: string[],
username: string,
labels: string[],
prTitle: string,
prDescription: string | undefined
) {
return sequentially(branches, async branch => {
Expand All @@ -35,6 +36,7 @@ export function doBackportVersions(
branch,
username,
labels,
prTitle,
prDescription
);
log(`View pull request: ${pullRequest.html_url}`);
Expand All @@ -56,6 +58,7 @@ export async function doBackportVersion(
baseBranch: string,
username: string,
labels: string[] = [],
prTitle: string,
prDescription: string | undefined
) {
const featureBranch = getFeatureBranchName(baseBranch, commits);
Expand Down Expand Up @@ -92,6 +95,7 @@ export async function doBackportVersion(
baseBranch,
commits,
username,
prTitle,
prDescription
);
const pullRequest = await createPullRequest(owner, repoName, payload);
Expand Down Expand Up @@ -171,19 +175,26 @@ async function resolveConflictsOrAbort(owner: string, repoName: string) {
}
}

function getPullRequestTitle(baseBranch: string, commits: Commit[]) {
const commitMessages = commits
function getPullRequestTitle(
baseBranch: string,
commits: Commit[],
prTitle: string
) {
// @ts-ignore
cjskillingstad marked this conversation as resolved.
Show resolved Hide resolved
const commitMessages = commits // eslint-disable-line @typescript-eslint/no-unused-vars
.map(commit => commit.message)
.join(' | ')
.slice(0, 200);

return `[${baseBranch}] ${commitMessages}`;
// prTitle could include baseBranch or commitMessages in template literal
return eval('`' + prTitle + '`');
Copy link
Owner

Choose a reason for hiding this comment

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

I think it's better to avoid eval and explicitly replacing certain tokens. Eg.

prTitle
  .replace('${baseBranch}', baseBranch)
  .replace('${commitMessages}', commitMessages)

It's not as powerful but it's explicit and will make it more obvious when breaking the contract with the end user. Plus, I'm afraid of eval 😱

Would this solution still work for you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this can definitely still work. I was weary of using eval in the first place. Fixed in fe109c0

Copy link
Owner

Choose a reason for hiding this comment

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

Looks great!

I noticed that $ is treated as a special character and everything after it will be stripped. So the following won't work: --prTitle="Custom title: ${commitMessages}"

This is a limitation with bash. Quick google turned up this and this.

Workarounds

  • Using single quotes : --prTitle='Custom title: ${commitMessages}'
  • Escaping: --prTitle="Custom title: \${commitMessages}"

To bypass the problem altogether it might be better to drop the $ and just use curly brackets - that way users won't have to resort to the workarounds above:

prTitle
  .replace('{baseBranch}', baseBranch)
  .replace('{commitMessages}', commitMessages)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, didn't think about that. Removed the dollar sign notation in
8e9bbe5

}

export function getPullRequestPayload(
baseBranch: string,
commits: Commit[],
username: string,
prTitle: string,
prDescription: string | undefined
) {
const featureBranch = getFeatureBranchName(baseBranch, commits);
Expand All @@ -197,7 +208,7 @@ export function getPullRequestPayload(
const bodySuffix = prDescription ? `\n\n${prDescription}` : '';

return {
title: getPullRequestTitle(baseBranch, commits),
title: getPullRequestTitle(baseBranch, commits, prTitle),
body: `Backports the following commits to ${baseBranch}:\n${commitRefs}${bodySuffix}`,
head: `${username}:${featureBranch}`,
base: `${baseBranch}`
Expand Down
1 change: 1 addition & 0 deletions src/steps/steps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export async function initSteps(options: BackportOptions) {
branches,
options.username,
options.labels,
options.prTitle,
options.prDescription
);
}
2 changes: 2 additions & 0 deletions test/options/cliArgs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ describe('getOptionsFromCliArgs', () => {
multiple: false,
multipleBranches: true,
multipleCommits: false,
prTitle: 'myPrTitle',
upstream: 'elastic/kibana',
username: 'sqren'
};
Expand Down Expand Up @@ -43,6 +44,7 @@ describe('getOptionsFromCliArgs', () => {
multiple: false,
multipleBranches: true,
multipleCommits: false,
prTitle: 'myPrTitle',
sha: undefined,
upstream: 'sqren/backport-demo',
username: 'sqren'
Expand Down
1 change: 1 addition & 0 deletions test/options/config/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ describe('getOptionsFromConfigFiles', () => {
multiple: false,
multipleBranches: true,
multipleCommits: false,
prTitle: '[${baseBranch}] ${commitMessages}',
upstream: 'elastic/kibana',
username: 'sqren'
});
Expand Down
1 change: 1 addition & 0 deletions test/options/options.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const validOptions = {
multiple: false,
multipleBranches: true,
multipleCommits: false,
prTitle: 'myPrTitle',
prDescription: undefined,
sha: undefined,
upstream: 'elastic/kibana',
Expand Down
2 changes: 1 addition & 1 deletion test/steps/__snapshots__/steps.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ Object {
"patch": Array [],
"post": Array [
Object {
"data": "{\\"title\\":\\"[6.2] myCommitMessage\\",\\"body\\":\\"Backports the following commits to 6.2:\\\\n - myCommitMessage (#myPullRequestNumber)\\\\n\\\\nmyPrDescription\\",\\"head\\":\\"sqren:backport/6.2/pr-myPullRequestNumber\\",\\"base\\":\\"6.2\\"}",
"data": "{\\"title\\":\\"myPrTitle\\",\\"body\\":\\"Backports the following commits to 6.2:\\\\n - myCommitMessage (#myPullRequestNumber)\\\\n\\\\nmyPrDescription\\",\\"head\\":\\"sqren:backport/6.2/pr-myPullRequestNumber\\",\\"base\\":\\"6.2\\"}",
"headers": Object {
"Accept": "application/json, text/plain, */*",
"Content-Type": "application/json;charset=utf-8",
Expand Down
3 changes: 3 additions & 0 deletions test/steps/doBackportVersions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ describe('doBackportVersion', () => {
'6.x',
'sqren',
['backport'],
'[${baseBranch}] ${commitMessages}',
'myPrSuffix'
);
});
Expand Down Expand Up @@ -107,6 +108,7 @@ describe('doBackportVersion', () => {
'6.x',
'sqren',
['backport'],
'[${baseBranch}] ${commitMessages}',
undefined
);
});
Expand Down Expand Up @@ -168,6 +170,7 @@ describe('doBackportVersion', () => {
'6.x',
'sqren',
['backport'],
'myPrTitle',
undefined
);

Expand Down
3 changes: 2 additions & 1 deletion test/steps/steps.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ describe('run through steps', () => {
multiple: false,
multipleBranches: false,
multipleCommits: false,
prTitle: 'myPrTitle',
prDescription: 'myPrDescription',
sha: undefined,
upstream,
Expand All @@ -185,7 +186,7 @@ describe('run through steps', () => {
base: '6.2',
body: `Backports the following commits to 6.2:\n - myCommitMessage (#myPullRequestNumber)\n\nmyPrDescription`,
head: 'sqren:backport/6.2/pr-myPullRequestNumber',
title: '[6.2] myCommitMessage'
title: 'myPrTitle'
});
});

Expand Down