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

[No QA] Improve GitUtils logging #31507

Merged
merged 4 commits into from
Nov 17, 2023
Merged

[No QA] Improve GitUtils logging #31507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7679483
Log repack command
roryabraham Nov 17, 2023
1589236
Sort PR list in GitUtils.getPullRequestsMergedBetween
roryabraham Nov 17, 2023
9fce76a
Rebuild gh actions
roryabraham Nov 17, 2023
97c1cb0
Update tests
roryabraham Nov 17, 2023
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
22 changes: 12 additions & 10 deletions .github/actions/javascript/createOrUpdateStagingDeploy/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,16 @@ function fetchTag(tag) {
let needsRepack = false;
while (shouldRetry) {
try {
let command = '';
if (needsRepack) {
// We have seen some scenarios where this fixes the git fetch.
// Why? Who knows... https://github.com/Expensify/App/pull/31459
execSync('git repack -d');
command = 'git repack -d';
console.log(`Running command: ${command}`);
execSync(command);
}

let command = `git fetch origin tag ${tag} --no-tags`;
command = `git fetch origin tag ${tag} --no-tags`;

// Exclude commits reachable from the previous patch version (i.e: previous checklist),
// so that we don't have to fetch the full history
Expand Down Expand Up @@ -315,16 +318,15 @@ function getValidMergedPRs(commits) {
* @param {String} toTag
* @returns {Promise<Array<Number>>} – Pull request numbers
*/
function getPullRequestsMergedBetween(fromTag, toTag) {
async function getPullRequestsMergedBetween(fromTag, toTag) {
console.log(`Looking for commits made between ${fromTag} and ${toTag}...`);
return getCommitHistoryAsJSON(fromTag, toTag).then((commitList) => {
console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);
const commitList = await getCommitHistoryAsJSON(fromTag, toTag);
console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);

// Find which commit messages correspond to merged PR's
const pullRequestNumbers = getValidMergedPRs(commitList);
console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
return _.map(pullRequestNumbers, (prNum) => Number.parseInt(prNum, 10));
});
// Find which commit messages correspond to merged PR's
const pullRequestNumbers = getValidMergedPRs(commitList).sort((a, b) => a - b);
console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
return pullRequestNumbers;
}

module.exports = {
Expand Down
22 changes: 12 additions & 10 deletions .github/actions/javascript/getDeployPullRequestList/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,16 @@ function fetchTag(tag) {
let needsRepack = false;
while (shouldRetry) {
try {
let command = '';
if (needsRepack) {
// We have seen some scenarios where this fixes the git fetch.
// Why? Who knows... https://github.com/Expensify/App/pull/31459
execSync('git repack -d');
command = 'git repack -d';
console.log(`Running command: ${command}`);
execSync(command);
}

let command = `git fetch origin tag ${tag} --no-tags`;
command = `git fetch origin tag ${tag} --no-tags`;

// Exclude commits reachable from the previous patch version (i.e: previous checklist),
// so that we don't have to fetch the full history
Expand Down Expand Up @@ -257,16 +260,15 @@ function getValidMergedPRs(commits) {
* @param {String} toTag
* @returns {Promise<Array<Number>>} – Pull request numbers
*/
function getPullRequestsMergedBetween(fromTag, toTag) {
async function getPullRequestsMergedBetween(fromTag, toTag) {
console.log(`Looking for commits made between ${fromTag} and ${toTag}...`);
return getCommitHistoryAsJSON(fromTag, toTag).then((commitList) => {
console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);
const commitList = await getCommitHistoryAsJSON(fromTag, toTag);
console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);

// Find which commit messages correspond to merged PR's
const pullRequestNumbers = getValidMergedPRs(commitList);
console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
return _.map(pullRequestNumbers, (prNum) => Number.parseInt(prNum, 10));
});
// Find which commit messages correspond to merged PR's
const pullRequestNumbers = getValidMergedPRs(commitList).sort((a, b) => a - b);
console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
return pullRequestNumbers;
}

module.exports = {
Expand Down
22 changes: 12 additions & 10 deletions .github/libs/GitUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ function fetchTag(tag) {
let needsRepack = false;
while (shouldRetry) {
try {
let command = '';
if (needsRepack) {
// We have seen some scenarios where this fixes the git fetch.
// Why? Who knows... https://github.com/Expensify/App/pull/31459
execSync('git repack -d');
command = 'git repack -d';
console.log(`Running command: ${command}`);
execSync(command);
}

let command = `git fetch origin tag ${tag} --no-tags`;
command = `git fetch origin tag ${tag} --no-tags`;

// Exclude commits reachable from the previous patch version (i.e: previous checklist),
// so that we don't have to fetch the full history
Expand Down Expand Up @@ -130,16 +133,15 @@ function getValidMergedPRs(commits) {
* @param {String} toTag
* @returns {Promise<Array<Number>>} – Pull request numbers
*/
function getPullRequestsMergedBetween(fromTag, toTag) {
async function getPullRequestsMergedBetween(fromTag, toTag) {
console.log(`Looking for commits made between ${fromTag} and ${toTag}...`);
return getCommitHistoryAsJSON(fromTag, toTag).then((commitList) => {
console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);
const commitList = await getCommitHistoryAsJSON(fromTag, toTag);
console.log(`Commits made between ${fromTag} and ${toTag}:`, commitList);

// Find which commit messages correspond to merged PR's
const pullRequestNumbers = getValidMergedPRs(commitList);
console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
return _.map(pullRequestNumbers, (prNum) => Number.parseInt(prNum, 10));
});
// Find which commit messages correspond to merged PR's
const pullRequestNumbers = getValidMergedPRs(commitList).sort((a, b) => a - b);
console.log(`List of pull requests merged between ${fromTag} and ${toTag}`, pullRequestNumbers);
return pullRequestNumbers;
}

module.exports = {
Expand Down
14 changes: 7 additions & 7 deletions tests/unit/CIGitLogicTest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ cherry_pick_pr 3
tag_staging

# Verify output for checklist
assert_prs_merged_between '1.0.0-0' '1.0.0-2' "[ 3, 1 ]"
assert_prs_merged_between '1.0.0-0' '1.0.0-2' "[ 1, 3 ]"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just FWIW, this didn't just reverse the order. Before, it was in order of most recently merged to least recently merged. Now it's in ascending numerical order, which is much more predictable when looking at actual PR numbers


# Verify output for deploy comment
assert_prs_merged_between '1.0.0-1' '1.0.0-2' "[ 3 ]"
Expand All @@ -252,7 +252,7 @@ title "Scenario #4A: Run the production deploy"
update_production_from_staging

# Verify output for release body and production deploy comments
assert_prs_merged_between '1.0.0-0' '1.0.0-2' "[ 3, 1 ]"
assert_prs_merged_between '1.0.0-0' '1.0.0-2' "[ 1, 3 ]"

success "Scenario #4A completed successfully!"

Expand Down Expand Up @@ -284,7 +284,7 @@ update_staging_from_main
tag_staging

# Verify output for checklist
assert_prs_merged_between '1.0.0-2' '1.0.1-1' "[ 5, 2 ]"
assert_prs_merged_between '1.0.0-2' '1.0.1-1' "[ 2, 5 ]"

# Verify output for deploy comment
assert_prs_merged_between '1.0.1-0' '1.0.1-1' "[ 5 ]"
Expand All @@ -307,7 +307,7 @@ update_staging_from_main
tag_staging

# Verify output for checklist
assert_prs_merged_between '1.0.0-2' '1.0.1-2' "[ 6, 5, 2 ]"
assert_prs_merged_between '1.0.0-2' '1.0.1-2' "[ 2, 5, 6 ]"

# Verify output for deploy comment
assert_prs_merged_between '1.0.1-1' '1.0.1-2' "[ 6 ]"
Expand All @@ -329,7 +329,7 @@ update_staging_from_main
tag_staging

# Verify output for checklist
assert_prs_merged_between '1.0.0-2' '1.0.1-3' "[ 7, 6, 5, 2 ]"
assert_prs_merged_between '1.0.0-2' '1.0.1-3' "[ 2, 5, 6, 7 ]"

# Verify output for deploy comment
assert_prs_merged_between '1.0.1-2' '1.0.1-3' "[ 7 ]"
Expand Down Expand Up @@ -389,10 +389,10 @@ update_staging_from_main
tag_staging

# Verify production release list
assert_prs_merged_between '1.0.0-2' '1.0.1-4' "[ 9, 7, 6, 5, 2 ]"
assert_prs_merged_between '1.0.0-2' '1.0.1-4' "[ 2, 5, 6, 7, 9 ]"

# Verify PR list for the new checklist
assert_prs_merged_between '1.0.1-4' '1.0.2-0' "[ 10, 8 ]"
assert_prs_merged_between '1.0.1-4' '1.0.2-0' "[ 8, 10 ]"

### Cleanup
title "Cleaning up..."
Expand Down
Loading