Skip to content

Conversation

@stanislaw
Copy link
Member

@stanislaw stanislaw commented Nov 1, 2019

This feature is inspired by another pending PR: #431.

While I need this feature to implement LIT-based tests so that they do not collide with the existing test suite, I think this feature can be useful in general because it makes it possible to skip useless directories recursively.

Also I noticed that Doorstop does os.walk through all of the .git, .venv, .cache and other directories which means it does a lot of useless work. I have added a filter for a at least .git folder to not be included into the os.walk traverse plan.

@stanislaw
Copy link
Member Author

Thanks for responding quickly! I need to write the unit tests to catch these coverage bits. Wanted to open this as a RFC first so it is good to have your approval in general!

@stanislaw
Copy link
Member Author

@jacebrowning I have written what I think are good tests but the coveralls still says I am decreasing the code coverage.

I am pretty sure it is due to the fact that the number of lines has increased, as @sebhub explained here: #425 (comment).

Any recommendations on how I should proceed?

I could grab any file with the lowest code coverage and open a separate PR to increase the total number. But before I do that I would like to understand where the setting for this check is located: is it a fixed number or the rule is "no less than" that the current value?

@jacebrowning
Copy link
Member

jacebrowning commented Nov 1, 2019

@stanislaw I just changed the coverage decrease threadshold to 0.1%, which is a setting on the Coveralls site:

I think the tests here are sufficient.

@stanislaw
Copy link
Member Author

@jacebrowning it has passed, thanks for the correction. Please proceed with a merge if everything is ok.

@jacebrowning jacebrowning merged commit 2226b89 into doorstop-dev:develop Nov 1, 2019
@stanislaw stanislaw deleted the check-doorstop-skip branch November 1, 2019 18:05
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