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.

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

Deid 1284 improve robustness of pii check on off flags #3

Merged
merged 4 commits into from
Nov 25, 2022

Conversation

ketakipai
Copy link
Contributor

@ketakipai ketakipai commented Nov 24, 2022

  • fixed logic so now it is compatible with all file types
  • works with whitespaces
  • tested on file mentioned in ticket and some xml and cpp files
    NOTE: I have updated the code in the testing repo (test-pre-commit) so you can test these changes by updating 'rev' in pre-commit-config.yaml to test.6 (see README)

Copy link
Contributor

@guyd guyd left a comment

Choose a reason for hiding this comment

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

Reviewed rapidly. LGTM. I made a minor comment to early exit the loop.

skip = False
for item in flagged:
if line > item[0] and line < item[1] and file == item[2]:
skip = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
skip = True
skip = True
break

@yaysummeriscoming
Copy link

@ketakipai could you hit me up on Signal once Guy's suggestion is in, and I'll retest on my end?

Copy link

@yaysummeriscoming yaysummeriscoming left a comment

Choose a reason for hiding this comment

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

Retested, works fine for me now thanks

@ketakipai ketakipai merged commit 492186f into dev Nov 25, 2022
@ketakipai ketakipai deleted the DEID-1284-improve-robustness-of-pii-check-on-off-flags branch November 25, 2022 06:38
ketakipai added a commit that referenced this pull request Dec 6, 2022
* added all files

* updated README

* Deid 1284 improve robustness of pii check on off flags (#3)

* fixed bugs with the PII_CHECK flags

* removed local hook

* added suggestion by Guy

* added Guys suggestion and removed local hook

* DEID-1311-update-git-pre-commit-hook-documentation (#4)

* updated README

* did the suggested changes

* Update the way API KEY is read (#2)

* passing path of env file as variable

* remove the unnecessary line from setup.py

* bug fix and updated script to get api key from env file

* updated README

* removed pathlib2 dependency and added stuff to check for relative path

* updated README

* added validation to check if API_KEY is missing

* updated README

* added all dependencies in setup.py, removed from config

* removing unnecessary dependecies from pre-commit-hooks

* updated dependencies

* removed unnecessary fields mentioned in README, tested and confirmed that we need the rest

* updated README

* updated README

* updated README

* updated pre-commit-hooks

* removing args from pre-commit-hooks

* removing entry from pre-commit-config

* updated README to resolve conflicts

* Make compatible with beta2 (#5)

* test

* updated fields to be compatible with beta 2

* removing line added  for debugging

* updated README to contain latest rev

* update README to have new repo

* added link_batch and removed an unused function (#6)

* added link_batch and removed an unused function

* removed test file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants