deprecate: replace --tidy-review/--format-review with --pr-review

- add `--pr-review` to control posting PR reviews
- make `--tidy-review`/`--format-review` aliases of `--pr-review` for backward compatibility

resolves #375

Author: 2bndy5

cpp-linter/src/clang_tools/clang_tidy.rs

@@ -518,8 +518,6 @@ mod test {
             database_json: None,
             format_filter: None,
             tidy_filter: None,
-            tidy_review: false,
-            format_review: false,
             clang_tidy_command: Some(exe_path),
             clang_format_command: None,
             repo_root: tmp_workspace.path().to_path_buf(),

cpp-linter/src/cli/mod.rs

@@ -506,31 +506,19 @@ pub struct FeedbackOptions {
     ))]
     pub file_annotations: bool,
 
-    /// Set to `true` to enable Pull Request reviews from clang-tidy.
+    /// Set to `true` to enable Pull Request reviews.
     #[cfg_attr(feature = "bin", arg(
-        short = 'd',
+        short = 'P',
         long,
         default_value_t = false,
         default_missing_value = "true",
         num_args = 0..=1,
         action = ArgAction::Set,
         value_parser = FalseyValueParser::new(),
         help_heading = "Feedback options",
+        aliases = ["format-review", "tidy-review"],
     ))]
-    pub tidy_review: bool,
-
-    /// Set to `true` to enable Pull Request reviews from clang-format.
-    #[cfg_attr(feature = "bin", arg(
-        short = 'm',
-        long,
-        default_value_t = false,
-        default_missing_value = "true",
-        num_args = 0..=1,
-        action = ArgAction::Set,
-        value_parser = FalseyValueParser::new(),
-        help_heading = "Feedback options",
-    ))]
-    pub format_review: bool,
+    pub pr_review: bool,
 
     /// Set to `true` to prevent Pull Request reviews from
     /// approving or requesting changes.

cpp-linter/src/cli/structs.rs

@@ -218,12 +218,6 @@ pub struct ClangParams {
     /// An optional [`FileFilter`] to exclude files only from clang-format analysis.
     pub format_filter: Option<FileFilter>,
 
-    /// Assert this if preparing a PR review with clang-tidy advice.
-    pub tidy_review: bool,
-
-    /// Assert this if preparing a PR review with clang-format advice.
-    pub format_review: bool,
-
     /// The root of the repository, used to locate relative file paths in processing.
     ///
     /// A project-specific cache folder is created in this path.
@@ -284,8 +278,6 @@ impl From<&Cli> for ClangParams {
             clang_format_command: None,
             tidy_filter,
             format_filter,
-            tidy_review: args.feedback_options.tidy_review,
-            format_review: args.feedback_options.format_review,
             repo_root,
         }
     }
@@ -309,11 +301,8 @@ pub struct FeedbackInput {
     /// The clang-format style to show in file annotations.
     pub style: String,
 
-    /// Whether to post a PR review with clang-tidy suggestions/notes.
-    pub tidy_review: bool,
-
-    /// Whether to post a PR review with clang-format suggestions.
-    pub format_review: bool,
+    /// Whether to post a PR review.
+    pub pr_review: bool,
 
     /// Should PR reviews be commentary?
     ///
@@ -334,8 +323,7 @@ impl From<&Cli> for FeedbackInput {
             step_summary: args.feedback_options.step_summary,
             thread_comments: args.feedback_options.thread_comments.clone(),
             file_annotations: args.feedback_options.file_annotations,
-            tidy_review: args.feedback_options.tidy_review,
-            format_review: args.feedback_options.format_review,
+            pr_review: args.feedback_options.pr_review,
             passive_reviews: args.feedback_options.passive_reviews,
             repo_root: args.source_options.repo_root.clone(),
         }
@@ -351,8 +339,7 @@ impl Default for FeedbackInput {
             step_summary: false,
             file_annotations: true,
             style: "llvm".to_string(),
-            tidy_review: false,
-            format_review: false,
+            pr_review: false,
             passive_reviews: false,
             repo_root: PathBuf::from("."),
         }

cpp-linter/src/rest_client.rs

@@ -183,9 +183,7 @@ impl RestClient {
             };
             self.client.post_thread_comment(options).await?;
         }
-        if self.client.is_pr_event()
-            && (feedback_inputs.tidy_review || feedback_inputs.format_review)
-        {
+        if self.client.is_pr_event() && feedback_inputs.pr_review {
             let summary_only = ["true", "on", "1"].contains(
                 &env::var("CPP_LINTER_PR_REVIEW_SUMMARY_ONLY")
                     .unwrap_or("false".to_string())

cpp-linter/src/run.rs

@@ -123,7 +123,7 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
                 FileObj::new(file_path)
             })
             .collect();
-        if is_pr && (cli.feedback_options.tidy_review || cli.feedback_options.format_review) {
+        if is_pr && cli.feedback_options.pr_review {
             let changed_files = rest_api_client
                 .get_list_of_changed_files(
                     &file_filter,
@@ -152,7 +152,7 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
     }
     rest_api_client.end_log_group("Get list of specified source files");
 
-    let mut clang_params = ClangParams::from(&cli);
+    let clang_params = ClangParams::from(&cli);
     // mkdir -p .cpp-linter-cache/
     let cache_dir = clang_params.repo_root.join(ClangParams::CACHE_DIR);
     std::fs::create_dir_all(&cache_dir)
@@ -163,8 +163,6 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
         "# Automatically created by cpp-linter\n*\n",
     )
     .with_context(|| "Failed to write .cpp-linter-cache/.gitignore file")?;
-    clang_params.format_review &= is_pr;
-    clang_params.tidy_review &= is_pr;
     let user_inputs = FeedbackInput::from(&cli);
     let clang_versions = capture_clang_tools_output(
         &arc_files,

cpp-linter/tests/reviews.rs

@@ -23,8 +23,7 @@ const REMAINING_RATE_LIMIT_HEADER: &str = "x-ratelimit-remaining";
 
 struct TestParams {
     pub lines_changed_only: LinesChangedOnly,
-    pub tidy_review: bool,
-    pub format_review: bool,
+    pub pr_review: bool,
     pub passive_reviews: bool,
     pub no_lgtm: bool,
     pub force_lgtm: bool,
@@ -42,8 +41,7 @@ impl Default for TestParams {
     fn default() -> Self {
         Self {
             lines_changed_only: LinesChangedOnly::On,
-            tidy_review: true,
-            format_review: true,
+            pr_review: true,
             passive_reviews: false,
             no_lgtm: true,
             force_lgtm: false,
@@ -175,9 +173,7 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) {
             "REQUEST_CHANGES"
         };
         let tool_summary = {
-            if !(test_params.format_review || test_params.tidy_review)
-                || test_params.lines_changed_only == LinesChangedOnly::On
-            {
+            if !test_params.pr_review || test_params.lines_changed_only == LinesChangedOnly::On {
                 // results can be non-deterministic because different clang-versions output different diagnostic/changes
                 ".*" // match anything
             } else if test_params.force_lgtm {
@@ -226,24 +222,22 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) {
         format!("-l={}", test_params.lines_changed_only),
         format!("--ignore-tidy={}", tidy_ignore),
         format!("--ignore-format={}", format_ignore),
-        format!("--tidy-review={}", test_params.tidy_review),
-        format!("--format-review={}", test_params.format_review),
+        format!("--pr-review={}", test_params.pr_review),
         format!("--passive-reviews={}", test_params.passive_reviews),
         format!("--no-lgtm={}", test_params.no_lgtm),
         "-p=build".to_string(),
         "-i=build".to_string(),
         format!("--repo-root={}", tmp_dir.path().to_str().unwrap()),
     ];
     if test_params.force_lgtm {
-        if test_params.tidy_review {
+        if test_params.pr_review {
             // only use a check that doesn't trigger concern on test assets
             args.push("--tidy-checks=-*,bugprone-infinite-loop".to_string());
-        }
-        if test_params.format_review {
             // explicitly disable formatting using `DisableFormat: true`
             args.push("--style={DisableFormat: true}".to_string());
         }
     } else {
+        // use default --tidy-checks value
         args.push("--style=file".to_string()); // use .clang-format file
     }
     match run_main(args).await {
@@ -279,26 +273,6 @@ async fn all_lines() {
     .await;
 }
 
-#[tokio::test]
-async fn all_lines_tidy_only() {
-    test_review(&TestParams {
-        lines_changed_only: LinesChangedOnly::Off,
-        format_review: false,
-        ..Default::default()
-    })
-    .await;
-}
-
-#[tokio::test]
-async fn all_lines_format_only() {
-    test_review(&TestParams {
-        lines_changed_only: LinesChangedOnly::Off,
-        tidy_review: false,
-        ..Default::default()
-    })
-    .await;
-}
-
 #[tokio::test]
 async fn changed_lines() {
     test_review(&TestParams::default()).await;