Skip to content

Commit

Permalink
Merge bitcoin#30812: lint: Check for release note snippets in the wro…
Browse files Browse the repository at this point in the history
…ng folder

fa3a7eb lint: Check for release note snippets in the wrong folder (MarcoFalke)

Pull request description:

  It is a common mistake to place the snippets in the wrong folder, where they could be missed. For example bitcoin#30719 (review) or commit 84900ac.

  Fix all issues by adding a simple lint check.

  Can be tested by reverting a prior commit that violated the rule and then running the new check:

  ```
  git revert 35ef34e
  ( cd ./test/lint/test_runner/ && RUST_BACKTRACE=1 cargo run -- --lint=doc_release_note_snippets )

ACKs for top commit:
  l0rinc:
    ACK fa3a7eb
  TheCharlatan:
    Re-ACK fa3a7eb

Tree-SHA512: 65a13696178aa8f94daa12a767cc74861293c631c19da9ca23c0fd43cedd47e7928d0ef14ad9ad83a434c1ac0e006f5a632ba9679756e071dea65b3cbf927c2d
  • Loading branch information
fanquake committed Sep 5, 2024
2 parents d6a1b94 + fa3a7eb commit d661e2b
Showing 1 changed file with 42 additions and 3 deletions.
45 changes: 42 additions & 3 deletions test/lint/test_runner/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
use std::env;
use std::fs;
use std::io::ErrorKind;
use std::path::{Path, PathBuf};
use std::path::PathBuf;
use std::process::{Command, ExitCode, Stdio};

/// A possible error returned by any of the linters.
///
/// The error string should explain the failure type and list all violations.
type LintError = String;
type LintResult = Result<(), LintError>;
type LintFn = fn() -> LintResult;
Expand Down Expand Up @@ -45,6 +48,11 @@ fn get_linter_list() -> Vec<&'static Linter> {
name: "std_filesystem",
lint_fn: lint_std_filesystem
},
&Linter {
description: "Check that release note snippets are in the right folder",
name: "doc_release_note_snippets",
lint_fn: lint_doc_release_note_snippets
},
&Linter {
description: "Check that subtrees are pure subtrees",
name: "subtree",
Expand Down Expand Up @@ -125,20 +133,27 @@ fn parse_lint_args(args: &[String]) -> Vec<&'static Linter> {
}

/// Return the git command
///
/// Lint functions should use this command, so that only files tracked by git are considered and
/// temporary and untracked files are ignored. For example, instead of 'grep', 'git grep' should be
/// used.
fn git() -> Command {
let mut git = Command::new("git");
git.arg("--no-pager");
git
}

/// Return stdout
/// Return stdout on success and a LintError on failure, when invalid UTF8 was detected or the
/// command did not succeed.
fn check_output(cmd: &mut std::process::Command) -> Result<String, LintError> {
let out = cmd.output().expect("command error");
if !out.status.success() {
return Err(String::from_utf8_lossy(&out.stderr).to_string());
}
Ok(String::from_utf8(out.stdout)
.map_err(|e| format!("{e}"))?
.map_err(|e| {
format!("All path names, source code, messages, and output must be valid UTF8!\n{e}")
})?
.trim()
.to_string())
}
Expand Down Expand Up @@ -276,6 +291,30 @@ fs:: namespace, which has unsafe filesystem functions marked as deleted.
}
}

fn lint_doc_release_note_snippets() -> LintResult {
let non_release_notes = check_output(git().args([
"ls-files",
"--",
"doc/release-notes/",
":(exclude)doc/release-notes/*.*.md", // Assume that at least one dot implies a proper release note
]))?;
if non_release_notes.is_empty() {
Ok(())
} else {
Err(format!(
r#"
{}
^^^
Release note snippets and other docs must be put into the doc/ folder directly.
The doc/release-notes/ folder is for archived release notes of previous releases only. Snippets are
expected to follow the naming "/doc/release-notes-<PR number>.md".
"#,
non_release_notes
))
}
}

/// Return the pathspecs for whitespace related excludes
fn get_pathspecs_exclude_whitespace() -> Vec<String> {
let mut list = get_pathspecs_exclude_subtrees();
Expand Down

0 comments on commit d661e2b

Please sign in to comment.