Skip to content

Commit 90a5451

Browse files
committed
remove logic judging commits
Signed-off-by: xizheyin <[email protected]>
1 parent a45844c commit 90a5451

File tree

4 files changed

+2
-219
lines changed

4 files changed

+2
-219
lines changed

src/config.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -432,10 +432,6 @@ pub(crate) struct NoMentionsConfig {}
432432
#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
433433
#[serde(deny_unknown_fields)]
434434
pub(crate) struct BehindMasterConfig {
435-
/// The threshold of commits behind master to trigger a warning.
436-
/// Default is 100 if not specified.
437-
pub(crate) commits_behind_threshold: Option<usize>,
438-
439435
/// The threshold of days for parent commit age to trigger a warning.
440436
/// Default is 14 days if not specified.
441437
pub(crate) parent_age_threshold: Option<usize>,

src/github.rs

Lines changed: 0 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,52 +1077,6 @@ impl Issue {
10771077
Ok(())
10781078
}
10791079

1080-
/// Returns the comparison between the PR and the base branch.
1081-
/// `BranchComparison::commits` are commits behind the base branch.
1082-
///
1083-
/// Returns `None` if this is not a PR or if getting the comparison fails.
1084-
pub async fn branch_comparison(
1085-
&self,
1086-
client: &GithubClient,
1087-
) -> anyhow::Result<BranchComparison> {
1088-
let repo_info = match client.repository(&self.repository().full_repo_name()).await {
1089-
Ok(repo) => repo,
1090-
Err(e) => {
1091-
log::error!(
1092-
"Error getting repository info for PR #{}: {}",
1093-
self.number,
1094-
e
1095-
);
1096-
return Err(e);
1097-
}
1098-
};
1099-
1100-
let Some(base) = &self.base else {
1101-
return Err(anyhow::anyhow!(
1102-
"No base branch found for PR #{}",
1103-
self.number
1104-
));
1105-
};
1106-
1107-
let Some(head) = &self.head else {
1108-
return Err(anyhow::anyhow!(
1109-
"No head branch found for PR #{}",
1110-
self.number
1111-
));
1112-
};
1113-
1114-
// head ref is something like `username:branch-name`
1115-
let head_label = &head.label;
1116-
// base label is `rust-lang:master``
1117-
let base_label = &base.label;
1118-
1119-
let comparison = repo_info
1120-
.compare_branches(client, base_label, head_label)
1121-
.await?;
1122-
1123-
Ok(comparison)
1124-
}
1125-
11261080
/// Checks if the PR's parent commit is too old.
11271081
///
11281082
/// This determines if a PR needs updating by examining the first parent of the PR's head commit,
@@ -1335,7 +1289,6 @@ struct PullRequestEventFields {}
13351289

13361290
#[derive(Clone, Debug, serde::Deserialize)]
13371291
pub struct CommitBase {
1338-
pub label: String,
13391292
pub sha: String,
13401293
#[serde(rename = "ref")]
13411294
pub git_ref: String,
@@ -2099,28 +2052,6 @@ impl Repository {
20992052
.await
21002053
.with_context(|| format!("{} failed to get pulls for commit {sha}", self.full_name))
21012054
}
2102-
2103-
/// Compare two branches, possibly across repositories
2104-
///
2105-
/// Format for head and base:
2106-
/// - For same repo: "branch_name"
2107-
/// - For different repo: "owner:branch_name"
2108-
///
2109-
/// Example: compare_branches(client, "main", "user:feature-branch").await
2110-
/// This will compare user:feature-branch against the main branch of the current repo
2111-
/// Note: GitHub API has a limit of 250 commits per page. So, we fetch **one** page is enough
2112-
pub async fn compare_branches(
2113-
&self,
2114-
client: &GithubClient,
2115-
base: &str,
2116-
head: &str,
2117-
) -> anyhow::Result<BranchComparison> {
2118-
let url = format!("{}/compare/{}...{}", self.url(client), base, head);
2119-
client
2120-
.json(client.get(&url))
2121-
.await
2122-
.with_context(|| format!("Failed to compare branches: {} vs {}", base, head))
2123-
}
21242055
}
21252056

21262057
/// Information about a merge conflict on a PR.
@@ -3349,23 +3280,6 @@ impl Submodule {
33493280
}
33503281
}
33513282

3352-
#[derive(Debug, serde::Deserialize)]
3353-
pub struct BranchComparison {
3354-
pub url: String,
3355-
pub html_url: String,
3356-
pub permalink_url: String,
3357-
pub diff_url: String,
3358-
pub patch_url: String,
3359-
pub base_commit: GithubCommit,
3360-
pub merge_base_commit: GithubCommit,
3361-
pub status: String,
3362-
pub ahead_by: u32,
3363-
pub behind_by: u32,
3364-
pub total_commits: u32,
3365-
pub commits: Vec<GithubCommit>,
3366-
pub files: Option<Vec<PullRequestFile>>,
3367-
}
3368-
33693283
#[cfg(test)]
33703284
mod tests {
33713285
use super::*;

src/handlers/check_commits.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,21 +96,12 @@ pub(super) async fn handle(ctx: &Context, event: &Event, config: &Config) -> any
9696

9797
// Check if PR is behind master branch by a significant number of commits
9898
if let Some(behind_master) = &config.behind_master {
99-
let commits_behind_threshold = behind_master
100-
.commits_behind_threshold
101-
.unwrap_or(behind_master::DEFAULT_COMMITS_BEHIND_THRESHOLD);
102-
10399
let parent_age_threshold = behind_master
104100
.parent_age_threshold
105101
.unwrap_or(behind_master::DEFAULT_PARENT_AGE_THRESHOLD);
106102

107-
if let Some(warning) = behind_master::behind_master(
108-
parent_age_threshold,
109-
commits_behind_threshold,
110-
event,
111-
&ctx.github,
112-
)
113-
.await
103+
if let Some(warning) =
104+
behind_master::behind_master(parent_age_threshold, event, &ctx.github).await
114105
{
115106
warnings.push(warning);
116107
}
Lines changed: 0 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,13 @@
11
use crate::github::{GithubClient, IssuesEvent};
22
use tracing as log;
33

4-
/// Default threshold for the number of commits behind master to trigger a warning
5-
pub const DEFAULT_COMMITS_BEHIND_THRESHOLD: usize = 100;
6-
74
/// Default threshold for parent commit age in days to trigger a warning
85
pub const DEFAULT_PARENT_AGE_THRESHOLD: usize = 14;
96

107
/// Check if the PR is behind the main branch by a significant number of commits
118
/// or based on an old parent commit
129
pub async fn behind_master(
1310
age_threshold: usize,
14-
merge_commits_threshold: usize,
1511
event: &IssuesEvent,
1612
client: &GithubClient,
1713
) -> Option<String> {
@@ -21,22 +17,6 @@ pub async fn behind_master(
2117

2218
log::debug!("Checking if PR #{} is behind master", event.issue.number);
2319

24-
// Get the repository info to determine default branch
25-
let repo_info = match client
26-
.repository(&event.issue.repository().full_repo_name())
27-
.await
28-
{
29-
Ok(repo) => repo,
30-
Err(e) => {
31-
log::error!(
32-
"Error getting repository info for PR #{}: {}",
33-
event.issue.number,
34-
e
35-
);
36-
return None;
37-
}
38-
};
39-
4020
// First try the parent commit age check as it's more accurate
4121
match event
4222
.issue
@@ -74,103 +54,5 @@ It's recommended to update your branch according to the \
7454
}
7555
}
7656

77-
// Fall back to the commit count method
78-
// check only auto-merge and rollup-merge commits
79-
let comparison = match event.issue.branch_comparison(client).await {
80-
Ok(comparison) => comparison,
81-
Err(e) => {
82-
log::error!(
83-
"Error getting branch comparison for PR #{}: {}",
84-
event.issue.number,
85-
e
86-
);
87-
return None;
88-
}
89-
};
90-
91-
// Total commits behind master
92-
let total_behind_by = comparison.behind_by as usize;
93-
94-
// First check if we're not totaly behind by much, no need to filter commits to make the check faster
95-
if total_behind_by < merge_commits_threshold {
96-
return None;
97-
}
98-
99-
log::debug!(
100-
"PR #{} is {} commits behind {}. Counting auto-merge and rollup-merge commits...",
101-
event.issue.number,
102-
total_behind_by,
103-
repo_info.default_branch
104-
);
105-
106-
// Count only auto-merge and rollup commits
107-
let mut auto_merge_commits = Vec::new();
108-
let mut rollup_merge_commits = Vec::new();
109-
110-
for commit in &comparison.commits {
111-
let message = &commit.commit.message;
112-
if message.starts_with("Auto merge of #") {
113-
auto_merge_commits.push(commit);
114-
} else if message.starts_with("Rollup merge of #") {
115-
rollup_merge_commits.push(commit);
116-
}
117-
}
118-
119-
let merge_commits_count = auto_merge_commits.len() + rollup_merge_commits.len();
120-
121-
log::info!(
122-
"PR #{} is {} commits behind {} (only including {} auto-merge and {} rollup commits)",
123-
event.issue.number,
124-
merge_commits_count,
125-
repo_info.default_branch,
126-
auto_merge_commits.len(),
127-
rollup_merge_commits.len()
128-
);
129-
130-
// Log detailed information about the auto-merge and rollup commits
131-
for commit in &auto_merge_commits {
132-
log::trace!(
133-
"Auto-merge commit: {} - {}",
134-
commit.sha,
135-
first_line(&commit.commit.message)
136-
);
137-
}
138-
139-
for commit in &rollup_merge_commits {
140-
log::trace!(
141-
"Rollup commit: {} - {}",
142-
commit.sha,
143-
first_line(&commit.commit.message)
144-
);
145-
}
146-
147-
// If there are at least the threshold of merge commits, generate a warning
148-
if merge_commits_count >= merge_commits_threshold as usize {
149-
log::info!(
150-
"PR #{} has {} merge commits ({} auto-merge and {} rollup) behind {} (threshold: {})",
151-
event.issue.number,
152-
merge_commits_count,
153-
auto_merge_commits.len(),
154-
rollup_merge_commits.len(),
155-
repo_info.default_branch,
156-
merge_commits_threshold
157-
);
158-
159-
return Some(format!(
160-
"This PR is missing {} important merge commits from the `{}` branch ({} auto-merge and {} rollup commits). \
161-
It's recommended to update your branch according to the \
162-
[Rustc Dev Guide](https://rustc-dev-guide.rust-lang.org/contributing.html#keeping-your-branch-up-to-date).",
163-
merge_commits_count,
164-
repo_info.default_branch,
165-
auto_merge_commits.len(),
166-
rollup_merge_commits.len()
167-
));
168-
}
169-
17057
None
17158
}
172-
173-
// Helper function to get the first line of a commit message
174-
fn first_line(message: &str) -> &str {
175-
message.lines().next().unwrap_or("")
176-
}

0 commit comments

Comments
 (0)