feat: Add ability to define PR title #125
Conversation
src/steps/doBackportVersions.ts
Outdated
| .map(commit => commit.message) | ||
| .join(' | ') | ||
| .slice(0, 200); | ||
|
|
||
| return `[${baseBranch}] ${commitMessages}`; | ||
| // prTitle could include baseBranch or commitMessages in template literal | ||
| return eval('`' + prTitle + '`'); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Yes, this can definitely still work. I was weary of using eval in the first place. Fixed in fe109c0
There was a problem hiding this comment.
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)There was a problem hiding this comment.
Good catch, didn't think about that. Removed the dollar sign notation in
8e9bbe5
|
Hi @cjskillingstad, Thanks for the contribution. Much appreciated! |
|
This is great. Will make a new release shortly. |
|
Released in v4.5.4 |
Resolves issue #126
Example:
"prTitle": "${commitMessages} backport for ${baseBranch}""[${baseBranch}] ${commitMessages}")