Add filtering_lines param to show issues only in modified lines #30

ShivamPokhriyal · 2021-05-22T09:13:10Z

Addresses #18

ShivamPokhriyal · 2021-06-02T12:07:23Z

@barbosa Can you take a look at this?

barbosa · 2021-06-02T13:50:36Z

Hey @ShivamPokhriyal - thank you very much for working on this. Would you want to include a few unit tests guaranteeing the logic is correct too? I can guide you through if you need any help.

ShivamPokhriyal · 2021-06-02T14:20:20Z

Would you want to include a few unit tests guaranteeing the logic is correct too? I can guide you through if you need any help.

I had a quick look into the test setup and it seems like this is being used as the git diff.
So, I'm thinking of adding a xml containing lint issues for the files in that diff and then write tests using that xml. Is this the correct way?

barbosa · 2021-06-02T15:02:08Z

I wonder if this shouldn't be covering that case. May we need to double-check what that test is actually doing.

ShivamPokhriyal · 2021-06-03T06:46:37Z

I wonder if this shouldn't be covering that case. May we need to double-check what that test is actually doing.

Looked into this test case and now I'm wondering about the actual meaning of filtering parameter.
If filtering simply means that it'll list all the lint issues in the modified files(even the issues at lines which aren't changed but are present in the same file), then that test case makes sense, and we'll probably need to create another parameter which will warn issues only for modified lines(and not modified files).
Otherwise, the test case isn't enough, it tests that when filtering is used, lint issues of only modified files are listed, but doesn't check whether the issues are of modified lines or unmodified lines of the modified file.

So, should we modify the test or create another parameter?

ShivamPokhriyal · 2021-06-07T14:50:39Z

@barbosa bumping again

barbosa · 2021-06-07T14:57:09Z

@ShivamPokhriyal I'd say it would be better to have another param as the original filtering one states it is for modified files.

Checking "lines" only would have to have higher priority than "files" though.

ShivamPokhriyal · 2021-06-07T15:02:29Z

Makes sense! Any ideas on the name of the new parameter?

barbosa · 2021-06-07T15:08:57Z

filtering_lines?

…

On Mon, Jun 7, 2021 at 11:02 AM Shivam Pokhriyal ***@***.***> wrote: Makes sense! Any ideas on the name of the new parameter? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#30 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABZNSDV5UROZSANCH6JTR3TRTNRXANCNFSM45KONVOQ> .

ShivamPokhriyal · 2021-06-12T07:56:19Z

spec/fixtures/pr-diff.diff

@@ -0,0 +1,14 @@
+diff --git a/app/build.gradle b/app/build.gradle


Using this PR https://github.com/dimagi/commcare-android/pull/2500/files for diff

ShivamPokhriyal · 2021-06-12T08:03:07Z

@barbosa Another round of review please.
I'm not sure how to run this plugin locally to test the changes, can you help me with that?

ShivamPokhriyal · 2021-06-12T18:03:59Z

I'm not sure how to run this plugin locally to test the changes, can you help me with that?

Got it, I just have to specify local path in gemfile.

barbosa · 2021-06-15T02:19:18Z

@ShivamPokhriyal did it work as expected when running with the changes locally? Lmk and I can merge this and submit a new version of the plugin.

ShivamPokhriyal · 2021-06-16T08:58:48Z

Ran it locally on this PR dimagi/commcare-android#2501 and it worked fine. @barbosa

➜  commcare-android git:(dummy-lint) ✗ bundle exec danger pr https://github.com/dimagi/commcare-android/pull/2501
Running your Dangerfile against this PR - https://github.com/dimagi/commcare-android/pull/2501
Turning on --verbose

Info:

+-------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|                                                                           Danger v8.2.3                                                                           |
|                                                                          DSL Attributes                                                                           |
+------------------+------------------------------------------------------------------------------------------------------------------------------------------------+
| violation_report |                                                                                                                                                |
|    status_report |                                                                                                                                                |
|              --- | ---                                                                                                                                            |
|     scm_provider | github                                                                                                                                         |
|             tags | #<Enumerator:0x00007f940b970328>                                                                                                               |
|          commits | b67bf14163879f9fb09208858b78318e65c45843                                                                                                       |
|      added_files | #<Danger::FileList:0x00007f940a9a3710>                                                                                                         |
|    deleted_files | #<Danger::FileList:0x00007f940a9a30d0>                                                                                                         |
|   modified_files | app/src/org/commcare/activities/AppManagerActivity.java                                                                                        |
|                  | app/src/org/commcare/activities/CommCareWiFiDirectActivity.java                                                                                |
|    renamed_files | []                                                                                                                                             |
|    lines_of_code | 32                                                                                                                                             |
|        deletions | 26                                                                                                                                             |
|       insertions | 6                                                                                                                                              |
|             diff | diff --git a/app/src/org/commcare/activities/AppManagerActivity.java b/app/src/org/commcare/activities/AppManagerActivity.java                 |
|                  | index 8be13f101..6299c3350 100644                                                                                                              |
|                  | --- a/app/src/org/commcare/activities/AppManagerActivity.java                                                                                  |
|                  | +++ b/app/src/org/commcare/activities/AppManagerActivity.java                                                                                  |
|                  | @@ -3,6 +3,7 @@ package org.commcare.activities;                                                                                               |
|                  |  import android.content.DialogInterface;                                                                                                       |
|                  |  import android.content.Intent;                                                                                                                |
|                  |  import android.os.Bundle;                                                                                                                     |
|                  | +import android.util.Log;                                                                                                                      |
|                  |  import android.view.Menu;                                                                                                                     |
|                  |  import android.view.MenuItem;                                                                                                                 |
|                  |  import android.view.View;                                                                                                                     |
|                  | @@ -47,6 +48,7 @@ public class AppManagerActivity extends CommCareActivity implements OnItemClickL                                             |
|                  |      @Override                                                                                                                                 |
|                  |      protected void onCreate(Bundle savedInstanceState) {                                                                                      |
|                  |          super.onCreate(savedInstanceState);                                                                                                   |
|                  | +        Log.d("Shivamm", "Should give lint issues");                                                                                          |
|                  |          setContentView(R.layout.app_manager);                                                                                                 |
|                  |          ((ListView)this.findViewById(R.id.apps_list_view)).setOnItemClickListener(this);                                                      |
|                  |          FirebaseAnalyticsUtil.reportAppManagerAction(AnalyticsParamValue.OPEN_APP_MANAGER);                                                   |
|                  | @@ -90,10 +92,7 @@ public class AppManagerActivity extends CommCareActivity implements OnItemClickL                                            |
|                  |      }                                                                                                                                         |
|                  |                                                                                                                                                |
|                  |      protected void showAboutCommCareDialog() {                                                                                                |
|                  | -        CommCareAlertDialog dialog = DialogCreationHelpers.buildAboutCommCareDialog(this, false);                                             |
|                  | -        dialog.makeCancelable();                                                                                                              |
|                  | -        dialog.setOnDismissListener(dialog1 -> handleDeveloperModeClicks());                                                                  |
|                  | -        showAlertDialog(dialog);                                                                                                              |
|                  | +        Log.d("Shivamm", "Another lint issue");                                                                                               |
|                  |      }                                                                                                                                         |
|                  |                                                                                                                                                |
|                  |      private void handleDeveloperModeClicks() {                                                                                                |
|                  | diff --git a/app/src/org/commcare/activities/CommCareWiFiDirectActivity.java b/app/src/org/commcare/activities/CommCareWiFiDirectActivity.java |
|                  | index f3f02913d..464f027e3 100644                                                                                                              |
|                  | --- a/app/src/org/commcare/activities/CommCareWiFiDirectActivity.java                                                                          |
|                  | +++ b/app/src/org/commcare/activities/CommCareWiFiDirectActivity.java                                                                          |
|                  | @@ -110,6 +110,7 @@ public class CommCareWiFiDirectActivity                                                                                    |
                                                                                        |
                                                                                                                                              |
                                                                                            |
                                              |"Adding lint issues in class which already have a lot of lint issues");
                                                                                                                                              |
                                                                                |t);
                                                                              |_text);
|                  | @@ -808,24 +809,7 @@ public class CommCareWiFiDirectActivity                                                                                   |
                                                                                                                                         |
                                                                                                                                              |
                                               |yFile(InputStream inputStream, OutputStream out) throws IOException {
                                                                                          |
                                                                                                            |
                                                                                             |
                                                                                   |
                                                                                                                                     |
                                                                                                          |
                                                                                                                              |
                                                                                                                                 |
                                                                                     |
                                                                                                       |
                                                                                                                                 |
                                                                                                                      |
                                                                                                              |
                                                                                                             |
                                                            |ile Server failed with exception " + e, e);
                                                                                                                          |
                                                                                                                                     |
                                                                                    |
                                                                                                       |
                                                                                                                                         |
                                                                                                                                              |
                                                                                                                                 |
|                  | @@ -836,10 +820,7 @@ public class CommCareWiFiDirectActivity                                                                                   |
                                                                                                                                              |
                                                                                                                                 |
                                                                                                               |
                                                                                       |
                                                    |Channel, (PeerListListener)this.getSupportFragmentManager()
                                                                                           |
                                                                                                                                              |
                                                                                     |
                                                                                                                                         |
                                                                                                                                              |
|                  |      @Override                                                                                                                                 |
|          mr_body | ## Summary                                                                                                                                     |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | <!--                                                                                                                                           |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | Provide a link to the ticket or document which prompted this change,                                                                           |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | Describe the rationale and design decisions.                                                                                                   |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | -->                                                                                                                                            |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | ## Feature Flag                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | <!-- If this is specific to a feature flag, which one? -->                                                                                     |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | ## Product Description                                                                                                                         |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | <!-- For non-invisible changes, describe user-facing effects. Would be good to a                                                               |
|                  | dd screenshots/videos for any major user facing changes -->                                                                                    |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | ## Safety Assurance                                                                                                                            |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | - [ ] If the PR is high risk, "High Risk" label is set                                                                                         |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | - [ ] I have confidence that this PR will not introduce a regression for the rea                                                               |
|                  | sons below                                                                                                                                     |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | - [ ] Do we need to enhance manual QA test coverage ? If yes, "QA Note" label is                                                               |
|                  | set correctly                                                                                                                                  |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | ### Automated test coverage                                                                                                                    |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | <!-- Identify the related test coverage and the tests it would catch -->                                                                       |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | ### Safety story                                                                                                                               |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | <!--                                                                                                                                           |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | Describe any other pieces to the safety story including                                                                                        |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | local testing, why the change is inherently safe, and/or plans to limit the blas                                                               |
|                  | t radius of a defect.                                                                                                                          |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | -->                                                                                                                                            |
|        mr_author | ShivamPokhriyal                                                                                                                                |
|        mr_labels | don't pull                                                                                                                                     |
|                  | skip-integration-tests                                                                                                                         |
|          pr_json | [Skipped JSON]                                                                                                                                 |
|          mr_json | [Skipped JSON]                                                                                                                                 |
|           review | #<Danger::RequestSources::GitHubSource::Review:0x00007f940a10d9e8>                                                                             |
|         pr_title | Dummy lint issue                                                                                                                               |
|          pr_body | ## Summary                                                                                                                                     |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | <!--                                                                                                                                           |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | Provide a link to the ticket or document which prompted this change,                                                                           |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | Describe the rationale and design decisions.                                                                                                   |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | -->                                                                                                                                            |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | ## Feature Flag                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | <!-- If this is specific to a feature flag, which one? -->                                                                                     |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | ## Product Description                                                                                                                         |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | <!-- For non-invisible changes, describe user-facing effects. Would be good to a                                                               |
|                  | dd screenshots/videos for any major user facing changes -->                                                                                    |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | ## Safety Assurance                                                                                                                            |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | - [ ] If the PR is high risk, "High Risk" label is set                                                                                         |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | - [ ] I have confidence that this PR will not introduce a regression for the rea                                                               |
|                  | sons below                                                                                                                                     |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | - [ ] Do we need to enhance manual QA test coverage ? If yes, "QA Note" label is                                                               |
|                  | set correctly                                                                                                                                  |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | ### Automated test coverage                                                                                                                    |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | <!-- Identify the related test coverage and the tests it would catch -->                                                                       |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | ### Safety story                                                                                                                               |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | <!--                                                                                                                                           |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | Describe any other pieces to the safety story including                                                                                        |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | local testing, why the change is inherently safe, and/or plans to limit the blas                                                               |
|                  | t radius of a defect.                                                                                                                          |
|                  |                                                                                                                                                |
|                  |                                                                                                                                                |
|                  | -->                                                                                                                                            |
|        pr_author | ShivamPokhriyal                                                                                                                                |
|        pr_labels | don't pull                                                                                                                                     |
|                  | skip-integration-tests                                                                                                                         |
|  branch_for_base | master                                                                                                                                         |
|  branch_for_head | dummy-lint                                                                                                                                     |
|      base_commit | 8dcbfeb44e774143428de579962def5038d77e8e                                                                                                       |
|      head_commit | b67bf14163879f9fb09208858b78318e65c45843                                                                                                       |
|              api | Octokit::Client                                                                                                                                |
|          pr_diff | [Skipped Diff]                                                                                                                                 |
|         mr_title | Dummy lint issue                                                                                                                               |
|        filtering |                                                                                                                                                |
|  filtering_lines |                                                                                                                                                |
|         severity | Warning                                                                                                                                        |
|      report_file | app/build/reports/lint/lint-result.xml                                                                                                         |
|      gradle_task | lint                                                                                                                                           |
| skip_gradle_task | false                                                                                                                                          |
|              --- | ---                                                                                                                                            |
|              SCM | Danger::GitRepo                                                                                                                                |
|           Source | Danger::LocalGitRepo                                                                                                                           |
|         Requests | Danger::RequestSources::GitHub                                                                                                                 |
|      Base Commit | commit 8dcbfeb44e774143428de579962def5038d77e8e                                                                                                |
|                  | Merge: 2410ea7ab fd97cc14c                                                                                                                     |
|                  | Author: Shivam Pokhriyal <[email protected]>                                                                                       |
|                  | Date:   Mon Jun 14 12:55:45 2021 +0530                                                                                                         |
|                  |                                                                                                                                                |
|                  |     Merge pull request #2499 from dimagi/refractor-draw-view                                                                                   |
|                  |                                                                                                                                                |
|                  |     Move DrawView to separate file                                                                                                             |
|      Head Commit | commit b67bf14163879f9fb09208858b78318e65c45843                                                                                                |
|                  | Author: ShivamPokhriyal <[email protected]>                                                                                        |
|                  | Date:   Wed Jun 16 13:38:19 2021 +0530                                                                                                         |
|                  |                                                                                                                                                |
|                  |     Dummy lint issue                                                                                                                           |
+------------------+------------------------------------------------------------------------------------------------------------------------------------------------+

Results:

Warnings:
- [ ] This PR does not contain any JIRA issue keys in the PR title or commit messages (e.g. KEY-123)
- [ ] app/src/org/commcare/activities/AppManagerActivity.java#L51: Using 'Log' instead of 'Timber'
- [ ] app/src/org/commcare/activities/AppManagerActivity.java#L95: Using 'Log' instead of 'Timber'
- [ ] app/src/org/commcare/activities/CommCareWiFiDirectActivity.java#L113: Using 'Log' instead of 'Timber'
- [ ] app/src/org/commcare/activities/CommCareWiFiDirectActivity.java#L812: Using 'Log' instead of 'Timber'
- [ ] app/src/org/commcare/activities/CommCareWiFiDirectActivity.java#L823: Using 'Log' instead of 'Timber'

ShivamPokhriyal · 2021-06-16T17:00:55Z

Just noticed that earlier I was using filtering_lines along with filtering. I updated my dangerfile to only have filtering_lines param and it gives some error. Here's my Dangerfile

android_lint.report_file = "app/build/reports/lint-results-commcareRelease.xml"
android_lint.skip_gradle_task = true
android_lint.filtering_lines = true
android_lint.lint(inline_mode: true)

The error I get is

bundler: failed to load command: danger (/Users/shivampokhriyal/.rbenv/versions/3.0.0/bin/danger)
/Users/shivampokhriyal/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/git-1.8.1/lib/git/diff.rb:64:in `[]': \e[31m (Danger::DSLError)
[!] Invalid `Dangerfile` file: undefined method `[]' for nil:NilClass\e[0m
 #  from Dangerfile:28
 #  -------------------------------------------
 #  android_lint.filtering = false
 >  android_lint.lint(inline_mode: true)
 #  -------------------------------------------
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:204:in `block (2 levels) in send_inline_comment'
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:198:in `each'
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:198:in `block in send_inline_comment'
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:195:in `each'
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:195:in `send_inline_comment'
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:122:in `lint'
	from Dangerfile:28:in `eval_file'

Will look more into it tomorrow. But just wanted to flag it here to not merge the PR yet.

ShivamPokhriyal · 2021-06-16T17:01:01Z

Just noticed that earlier I was using filtering_lines along with filtering. I updated my dangerfile to only have filtering_lines param and it gives some error. Here's my Dangerfile

android_lint.report_file = "app/build/reports/lint-results-commcareRelease.xml"
android_lint.skip_gradle_task = true
android_lint.filtering_lines = true
android_lint.lint(inline_mode: true)

The error I get is

bundler: failed to load command: danger (/Users/shivampokhriyal/.rbenv/versions/3.0.0/bin/danger)
/Users/shivampokhriyal/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/git-1.8.1/lib/git/diff.rb:64:in `[]': \e[31m (Danger::DSLError)
[!] Invalid `Dangerfile` file: undefined method `[]' for nil:NilClass\e[0m
 #  from Dangerfile:28
 #  -------------------------------------------
 #  android_lint.filtering = false
 >  android_lint.lint(inline_mode: true)
 #  -------------------------------------------
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:204:in `block (2 levels) in send_inline_comment'
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:198:in `each'
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:198:in `block in send_inline_comment'
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:195:in `each'
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:195:in `send_inline_comment'
	from /Users/shivampokhriyal/Documents/projects/danger-android_lint/lib/android_lint/plugin.rb:122:in `lint'
	from Dangerfile:28:in `eval_file'

Will look more into it tomorrow. But just wanted to flag it here to not merge the PR yet.

ShivamPokhriyal · 2021-06-16T17:16:37Z

The error I get is

Ohh nevermind, I think it was happening because git diff was using my local diff and not for PR. But now, with filtering_lines set to true and filtering set to false, it simply prints all the issues in the repo because of this check https://github.com/loadsmart/danger-android_lint/pull/30/files#diff-0d7e373d04e96e69ed2571afc6348abb2dfc5ff390982cc0bcc6e0f167cac92bR201 which always evaluates to true since filtering is false.

          next unless !filtering || (target_files.include? filename)

probably need to change that to something like next unless (!filtering && !filtering_lines) || (target_files.include? filename)
Will need to improve the test case as well.

…issues Test will currently fail because filtering_lines implementation is buggy currently

ShivamPokhriyal · 2021-06-17T05:32:52Z

@barbosa added the changes. Another round of review please.

ShivamPokhriyal · 2021-06-23T13:51:18Z

@barbosa bumping you again.

barbosa · 2021-06-23T15:22:43Z

This is now available on v0.0.9
Thank you for your contribution, @ShivamPokhriyal 👍

Warn lint issues for only added lines with filtering option

bc960e0

ShivamPokhriyal requested a review from a team as a code owner May 22, 2021 09:13

loadsmart deleted a comment from peril-loadsmart bot Jun 2, 2021

barbosa requested review from barbosa and removed request for a team June 7, 2021 14:57

ShivamPokhriyal added 3 commits June 12, 2021 10:33

Add filtering_lines option

fd3c7a1

Resolve cannot identify diff_for_file

2502827

Add tests for filtering_lines

130f308

ShivamPokhriyal commented Jun 12, 2021

View reviewed changes

ShivamPokhriyal changed the title ~~Warn lint issues for only added lines with filtering option~~ Add filtering_lines param to show issues only in modified lines Jun 12, 2021

Add changelog entry

efdd48b

ShivamPokhriyal added 2 commits June 17, 2021 10:34

Modify tests to assert filtering_lines doesn't show unmodified files …

eb8e3f1

…issues Test will currently fail because filtering_lines implementation is buggy currently

Fix filtering_lines implementation to conform to tests

44fe23c

barbosa approved these changes Jun 23, 2021

View reviewed changes

barbosa merged commit a53f731 into loadsmart:master Jun 23, 2021

ShivamPokhriyal deleted the sp/filtering-fix branch June 24, 2021 05:23

This was referenced Apr 18, 2022

Fix filtering_lines parameter to also apply when inline_mode is false #33

Merged

android_lint.filtering = true doesn't work #18

Open

errysuprayogi mentioned this pull request Aug 31, 2022

Add filtering_lines param to show issues only in modified lines NFesquet/danger-kotlin_detekt#16

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add filtering_lines param to show issues only in modified lines #30

Add filtering_lines param to show issues only in modified lines #30

ShivamPokhriyal commented May 22, 2021

ShivamPokhriyal commented Jun 2, 2021

barbosa commented Jun 2, 2021

ShivamPokhriyal commented Jun 2, 2021

barbosa commented Jun 2, 2021

ShivamPokhriyal commented Jun 3, 2021

ShivamPokhriyal commented Jun 7, 2021

barbosa commented Jun 7, 2021

ShivamPokhriyal commented Jun 7, 2021

barbosa commented Jun 7, 2021 via email

ShivamPokhriyal Jun 12, 2021

ShivamPokhriyal commented Jun 12, 2021

ShivamPokhriyal commented Jun 12, 2021

barbosa commented Jun 15, 2021

ShivamPokhriyal commented Jun 16, 2021

ShivamPokhriyal commented Jun 16, 2021

ShivamPokhriyal commented Jun 16, 2021

ShivamPokhriyal commented Jun 16, 2021 •

edited

Loading

ShivamPokhriyal commented Jun 17, 2021

ShivamPokhriyal commented Jun 23, 2021

barbosa commented Jun 23, 2021

		@@ -0,0 +1,14 @@
		diff --git a/app/build.gradle b/app/build.gradle

Add filtering_lines param to show issues only in modified lines #30

Add filtering_lines param to show issues only in modified lines #30

Conversation

ShivamPokhriyal commented May 22, 2021

ShivamPokhriyal commented Jun 2, 2021

barbosa commented Jun 2, 2021

ShivamPokhriyal commented Jun 2, 2021

barbosa commented Jun 2, 2021

ShivamPokhriyal commented Jun 3, 2021

ShivamPokhriyal commented Jun 7, 2021

barbosa commented Jun 7, 2021

ShivamPokhriyal commented Jun 7, 2021

barbosa commented Jun 7, 2021 via email

ShivamPokhriyal Jun 12, 2021

Choose a reason for hiding this comment

ShivamPokhriyal commented Jun 12, 2021

ShivamPokhriyal commented Jun 12, 2021

barbosa commented Jun 15, 2021

ShivamPokhriyal commented Jun 16, 2021

ShivamPokhriyal commented Jun 16, 2021

ShivamPokhriyal commented Jun 16, 2021

ShivamPokhriyal commented Jun 16, 2021 • edited Loading

ShivamPokhriyal commented Jun 17, 2021

ShivamPokhriyal commented Jun 23, 2021

barbosa commented Jun 23, 2021

ShivamPokhriyal commented Jun 16, 2021 •

edited

Loading