Skip to content

Do not include CP merge commits in the list of PR merge commits #6165

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

Merged
roryabraham merged 16 commits into from
Nov 29, 2021
Merged

Do not include CP merge commits in the list of PR merge commits #6165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2550687
Filter out the commits introduced by cherry picking
mountiny Nov 1, 2021
4664a8a
Add comment
mountiny Nov 1, 2021
62799cf
Update the regex and format of log
mountiny Nov 1, 2021
11e7580
Update tests
mountiny Nov 1, 2021
d283a83
Update gitutils git log tests
mountiny Nov 1, 2021
b911b88
Update unit test and regex for cherry pick commit message
mountiny Nov 1, 2021
4bda84c
Build the Github Actions
mountiny Nov 1, 2021
76d2ab6
Merge branch 'main' into vit-updateMergeCommitRegex
mountiny Nov 24, 2021
f839e13
Use two regexp to parse the pr numbers
mountiny Nov 25, 2021
aa1b134
Remove whitespace
mountiny Nov 25, 2021
b1fa8c6
Build the actions
mountiny Nov 25, 2021
69a8816
Use arrow function
mountiny Nov 25, 2021
6106845
Update logs
mountiny Nov 25, 2021
7529734
Adjust the array to not overdo the max char per line limit
mountiny Nov 25, 2021
c94553e
Build the actions
mountiny Nov 25, 2021
3c6d0dd
Fix linter
mountiny Nov 25, 2021
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
25 changes: 22 additions & 3 deletions .github/actions/createOrUpdateStagingDeploy/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,33 @@ const {execSync} = __nccwpck_require__(3129);
* @returns {Array}
*/
function getPullRequestsMergedBetween(fromRef, toRef) {
const command = `git log --format="%s" ${fromRef}...${toRef}`;
const command = `git log --format="{[%B]}" ${fromRef}...${toRef}`;
console.log('Getting pull requests merged between the following refs:', fromRef, toRef);
console.log('Running command: ', command);
const localGitLogs = execSync(command).toString();
return _.map(
[...localGitLogs.matchAll(/Merge pull request #(\d{1,6}) from (?!Expensify\/(?:master|main|version-))/g)],

// Parse the git log into an array of commit messages between the two refs
const commitMessages = _.map(
[...localGitLogs.matchAll(/{\[([\s\S]*?)\]}/gm)],
Comment on lines +203 to +205

@mountiny mountiny Nov 25, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have used your advice and split it up to two regexps. Here, I get the individual commit messages which are all inside of the {[]}.

match => match[1],
);
console.log(`A list of commits made between ${fromRef} and ${toRef}:\n${commitMessages}`);

// We need to find which commit messages correspond to merge commits and get PR numbers.
// Additionally, we omit merge commits made while cherry picking using negative lookahead in the regexp.
const pullRequestIDs = _.reduce(commitMessages, (mergedPRs, commitMessage) => {
const mergeCommits = [
...commitMessage.matchAll(/Merge pull request #(\d{1,6}) from (?!(?:Expensify\/(?:master|main|version-))|(?:([\s\S]*?)\(cherry picked from commit .*\)\s*))/gm),
];

// Get the PR number of the first match (there should not be multiple matches in one commit message)
if (_.size(mergeCommits)) {
mergedPRs.push(mergeCommits[0][1]);
}
return mergedPRs;
}, []);
console.log(`A list of pull requests merged between ${fromRef} and ${toRef}:\n${pullRequestIDs}`);
return pullRequestIDs;
}

module.exports = {
Expand Down
25 changes: 22 additions & 3 deletions .github/actions/getDeployPullRequestList/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,33 @@ const {execSync} = __nccwpck_require__(3129);
* @returns {Array}
*/
function getPullRequestsMergedBetween(fromRef, toRef) {
const command = `git log --format="%s" ${fromRef}...${toRef}`;
const command = `git log --format="{[%B]}" ${fromRef}...${toRef}`;
console.log('Getting pull requests merged between the following refs:', fromRef, toRef);
console.log('Running command: ', command);
const localGitLogs = execSync(command).toString();
return _.map(
[...localGitLogs.matchAll(/Merge pull request #(\d{1,6}) from (?!Expensify\/(?:master|main|version-))/g)],

// Parse the git log into an array of commit messages between the two refs
const commitMessages = _.map(
[...localGitLogs.matchAll(/{\[([\s\S]*?)\]}/gm)],
match => match[1],
);
console.log(`A list of commits made between ${fromRef} and ${toRef}:\n${commitMessages}`);

// We need to find which commit messages correspond to merge commits and get PR numbers.
// Additionally, we omit merge commits made while cherry picking using negative lookahead in the regexp.
const pullRequestIDs = _.reduce(commitMessages, (mergedPRs, commitMessage) => {
const mergeCommits = [
...commitMessage.matchAll(/Merge pull request #(\d{1,6}) from (?!(?:Expensify\/(?:master|main|version-))|(?:([\s\S]*?)\(cherry picked from commit .*\)\s*))/gm),
];

// Get the PR number of the first match (there should not be multiple matches in one commit message)
if (_.size(mergeCommits)) {
mergedPRs.push(mergeCommits[0][1]);
}
return mergedPRs;
}, []);
console.log(`A list of pull requests merged between ${fromRef} and ${toRef}:\n${pullRequestIDs}`);
return pullRequestIDs;
}

module.exports = {
Expand Down
25 changes: 22 additions & 3 deletions .github/libs/GitUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,33 @@ const {execSync} = require('child_process');
* @returns {Array}
*/
function getPullRequestsMergedBetween(fromRef, toRef) {
const command = `git log --format="%s" ${fromRef}...${toRef}`;
const command = `git log --format="{[%B]}" ${fromRef}...${toRef}`;
console.log('Getting pull requests merged between the following refs:', fromRef, toRef);
console.log('Running command: ', command);
const localGitLogs = execSync(command).toString();
return _.map(
[...localGitLogs.matchAll(/Merge pull request #(\d{1,6}) from (?!Expensify\/(?:master|main|version-))/g)],

// Parse the git log into an array of commit messages between the two refs
const commitMessages = _.map(
[...localGitLogs.matchAll(/{\[([\s\S]*?)\]}/gm)],
match => match[1],
);
console.log(`A list of commits made between ${fromRef} and ${toRef}:\n${commitMessages}`);

// We need to find which commit messages correspond to merge commits and get PR numbers.
// Additionally, we omit merge commits made while cherry picking using negative lookahead in the regexp.
const pullRequestIDs = _.reduce(commitMessages, (mergedPRs, commitMessage) => {
const mergeCommits = [
...commitMessage.matchAll(/Merge pull request #(\d{1,6}) from (?!(?:Expensify\/(?:master|main|version-))|(?:([\s\S]*?)\(cherry picked from commit .*\)\s*))/gm),
];

// Get the PR number of the first match (there should not be multiple matches in one commit message)
if (_.size(mergeCommits)) {
mergedPRs.push(mergeCommits[0][1]);
}
return mergedPRs;
}, []);
console.log(`A list of pull requests merged between ${fromRef} and ${toRef}:\n${pullRequestIDs}`);
return pullRequestIDs;
}

module.exports = {
Expand Down
117 changes: 85 additions & 32 deletions tests/unit/GitUtilsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,43 +6,96 @@ const {execSync} = childProcess;

const data = [
{
gitLog: `Start adding StagingDeployCash logic
Setting up bones
Merge pull request #337 from Expensify/francoisUpdateQbdSyncManager
Merge pull request #336 from Expensify/andrew-pr-cla
Update QBD Sync Manager version
Only run CLA on PR comments or events
Merge pull request #331 from Expensify/marcaaron-killMoment
Merge pull request #330 from Expensify/andrew-cla-update
Merge pull request #333 from Expensify/Rory-AddOnOffSwitchTooltip
Setup OnOffSwitch component with tooltips
Merge pull request #332 from Expensify/alex-mechler-patch-1
Return to old hash-based deploy instrcutions
Remove DEFAULT_START_DATE & DEFAULT_END_DATE altogether`,
gitLog: `{[Start adding StagingDeployCash logic
]}
{[Setting up bones
]}
{[Merge pull request #337 from Expensify/francoisUpdateQbdSyncManager
]}
{[Merge pull request #336 from Expensify/andrew-pr-cla
]}
{[Update QBD Sync Manager version
]}
{[Only run CLA on PR comments or events
]}
{[Merge pull request #331 from Expensify/marcaaron-killMoment
]}
{[Merge pull request #330 from Expensify/andrew-cla-update
]}
{[Merge pull request #333 from Expensify/Rory-AddOnOffSwitchTooltip
]}
{[Setup OnOffSwitch component with tooltips
]}
{[Merge pull request #332 from Expensify/alex-mechler-patch-1
]}
{[Return to old hash-based deploy instrcutions
]}
{[Remove DEFAULT_START_DATE & DEFAULT_END_DATE altogether
]}`,
result: ['337', '336', '331', '330', '333', '332'],
},
{
gitLog: `Merge pull request #1521 from parasharrajat/parasharrajat/pdf-render
Merge pull request #1563 from Expensify/version-bump-e6498075e301df3e9c8d7866ea391a23c19ed9b0
Update version to 1.0.1-470
Merge pull request #1557 from aliabbasmalik8/IS-1500-compose-field-alignment-issue
Merge pull request #1562 from Expensify/version-bump-b9c85aa97dfb656b01a83871b4bbaed5e287c8b7
Update version to 1.0.1-469
Merge pull request #1515 from anthony-hull/typos
[IS-1500] Updated textalignInput utility
Merge pull request #1560 from Expensify/version-bump-b742a55d18e761cd7adb0849a29cfb48b3a04f99
Update version to 1.0.1-468
Merge pull request #1555 from SameeraMadushan/sameera-IsAppInstalledLogic
Merge pull request #1 from Expensify/master
Merge pull request #2 from Expensify/main
fix: set pdf width on large screens
[IS-1500] Fixed compose field alignment issue`,
result: ['1521', '1557', '1515', '1555'],
gitLog: `{[Merge pull request #1521 from parasharrajat/parasharrajat/pdf-render
]}
{[Merge pull request #1563 from Expensify/version-bump-e6498075e301df3e9c8d7866ea391a23c19ed9b0
]}
{[Update version to 1.0.1-470
]}
{[Merge pull request #1557 from aliabbasmalik8/IS-1500-compose-field-alignment-issue
]}
{[Merge pull request #1562 from Expensify/version-bump-b9c85aa97dfb656b01a83871b4bbaed5e287c8b7
]}
{[Update version to 1.0.1-469
]}
{[Merge pull request #1515 from anthony-hull/typos
]}
{[[IS-1500] Updated textalignInput utility
]}
{[Merge pull request #1560 from Expensify/version-bump-b742a55d18e761cd7adb0849a29cfb48b3a04f99
]}
{[Update version to 1.0.1-468
]}
{[Merge pull request #1555 from SameeraMadushan/sameera-IsAppInstalledLogic
]}
{[Merge pull request #1 from Expensify/master
]}
{[Merge pull request #2 from Expensify/main
]}
{[fix: set pdf width on large screens
]}
{[[IS-1500] Fixed compose field alignment issue
]}
{[Merge pull request #5812 from akshayasalvi/tooltip-workspace-name …
42b5016
Added tooltip for workspace name

(cherry picked from commit a77d468)
]}
{[Merge pull request #5813 from Expensify/master …
42b5016
Added tooltip for workspace name

(cherry picked from commit a77d468)
]}
{[Merge pull request #1111 from akshayasalvi/tooltip-workspace-name …
42b5016
Added tooltip for workspace name

(something else than cherry pick commit a77d468)
]}
{[Merge pull request #9 from Expensify/main
]}
{[fix: set pdf width on large screens
]}`,
result: ['1521', '1557', '1515', '1555', '1111'],
},
{
gitLog: `Return to old hash-based deploy instrcutions
Remove DEFAULT_START_DATE & DEFAULT_END_DATE altogether
refactor`,
gitLog: `{[Return to old hash-based deploy instrcutions
]}
{[Remove DEFAULT_START_DATE & DEFAULT_END_DATE altogether
]}
{[refactor
]}`,
result: [],
},
];
Expand Down