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

Fix exclude_regex functionality (Issue 17) #45

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix exclude_regex functionality (Issue 17) #45

wants to merge 7 commits into from

Conversation

sp3nx0r
Copy link
Contributor

@sp3nx0r sp3nx0r commented Jan 22, 2020

Addresses #17 and #44

Removes exclude_regex from tracked repo metadata and config yaml add and replaces with an exclude_files_regex and exclude_lines_regex. CLI args are appended to the repo args (but are not overwritten in tracked repo metadata).

@KevinHock
Copy link
Collaborator

Thanks so much for making this! 🙏 I will take a look at it this weekend

@KevinHock
Copy link
Collaborator

(Gonna review after Tavis passes
currently get a

            self.add_non_local_repo(mock_rootdir)
    
>       assert not mock_file_operations.write.called
E       AssertionError: assert not True
E        +  where True = <MagicMock name='open().write' id='139948498142936'>.called
E        +    where <MagicMock name='open().write' id='139948498142936'> = <MagicMock name='open()' id='139948498142488'>.write
tests/actions/initialize_test.py:243: AssertionError
===================== 1 failed, 97 passed in 1.22 seconds ======================

I think)

@sp3nx0r
Copy link
Contributor Author

sp3nx0r commented Jan 28, 2020

(Gonna review after Tavis passes
currently get a

            self.add_non_local_repo(mock_rootdir)
    
>       assert not mock_file_operations.write.called
E       AssertionError: assert not True
E        +  where True = <MagicMock name='open().write' id='139948498142936'>.called
E        +    where <MagicMock name='open().write' id='139948498142936'> = <MagicMock name='open()' id='139948498142488'>.write
tests/actions/initialize_test.py:243: AssertionError
===================== 1 failed, 97 passed in 1.22 seconds ======================

I think)

Similar test case modification for the empty repo PR #46. Will open a new issue to handle overriding globally because maybe someone wants to not always override?

Copy link
Collaborator

@KevinHock KevinHock left a comment

Choose a reason for hiding this comment

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

lgtm 🚢 :shipit:

Thanks so much for making this!

I think there is a merge conflict that needs to be resolve, but that's it.

@sp3nx0r
Copy link
Contributor Author

sp3nx0r commented Feb 3, 2020

Looks like this PR pushed us under 93% coverage in Travis:

ERROR: InvocationError for command /home/travis/build/Yelp/detect-secrets-server/.tox/pypy/bin/coverage report --show-missing '--include=detect_secrets_server/*' --fail-under 93 (exited with code 2)

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.

2 participants