Earlier, there was a flake8 workflow error, and a contributor mentioned in a previous issue that that was caused due to a longer line, so I reduced the complexity of this code. Hope the workflow tests pass.

Hello , I am kind of confused.. The complexity is reduced but ultimately it is the same thing so I am not sure on how did it pass the checks? I am new so I am willing to learn more 🙂

Actually I simply had a flake8 workflow error. Just like you, I am just getting used to contributing to mypy as well. So a contributor in a previous issue stated that reducing the horizontal line size would suffice I guess. But in that case, I had 1 long line that comprehended the list with loops inside it. I simply broke the list and separated out the for loop. You can refer to this PR #11453 to track the history where I made the changes to tackle the flake8 error. So, even in this case, I tried reducing the length of the line, hoping this works!

Hello , I am kind of confused.. The complexity is reduced but ultimately it is the same thing so I am not sure on how did it pass the checks? I am new so I am willing to learn more 🙂

Sorry, actually there was a trailing whitespace in a line, I hope it passes the flake8 workflow test now.

@JukkaL @ethanhs @JelleZijlstra
It would be really great if someone initiated the workflow tests on this commit.
Thanks and sorry for the inconvenience.

There is one problem though.. I think original issue post wanted to print the message only for blacklisted module paths but your code will print for "None" module paths as well.. I can be wrong but just wanted to point it out 🙂

There is one problem though.. I think original issue post wanted to print the message only for blacklisted module paths but your code will print for "None" module paths as well.. I can be wrong but just wanted to point it out 🙂

You're right, I just committed the new changes, thanks!

Just had a quick question for the other contributors. When are the Pull Requests that have passed all checks merged to the master repository? Should I @ someone to review my changes? (I also have another issue resolved a few days back)
@JukkaL @ethanhs @hauntsaninja Please reach out.
Thanks in advanced!

Just had a quick question for the other contributors. When are the Pull Requests that have passed all checks merged to the master repository? Should I @ someone to review my changes? (I also have another issue resolved a few days back) @JukkaL @ethanhs @hauntsaninja Please reach out. Thanks in advanced!

I am also not sure because I am waiting for my PR to get reviewed on the same issue (it passed the checks as well)

@SwagatSBhuyan just in case it wasn't clear, though I assume it probably is because you both commented on #9599 and @Infinil had already said they were going to create a PR and created #11476 before you did. It would make sense to let @Infinil drive their PR to completion rather than requesting a review of your PR in order to merge yours before @Infinil's which already got an initial review.

@Infinil I'd suggest in the future to kindly comment (i.e. in your initial message) that you had already created a PR that does the same thing. You vaguely say so, but you don't specify where it is. I see @SwagatSBhuyan could have looked on #9599 to see the link, but sharing it here and not burying what "it is the same thing" will make it clearer to everyone that there is another PR which may also be older (it's fine if it wasn't and you accidentally ended up implementing the same thing (I've done that myself)).

Regarding this PR @SwagatSBhuyan, it does look like #11476 handles module.path is None the same way as the pre-existing code while this one does not. I'm not sure why module.path would be None and it looks like the CI is green on both of your PRs, but it's something to call out because you're changing the behavior (possibly in an OK way). My guess is a dynamic or eval'ed module may have no path 🤔. Also, a comment on your PR title. I would suggest in the future you make sure the PR title describes the change you are making not just what you're fixing (since the issue title is not actually the problem, and you are not making it not skip the vendor directory). #11476 is a pretty good example of what it could have been, because both of your changes are printing what modules are skipped which allows a user to understand why vendor would be ignored.

@SwagatSBhuyan just in case it wasn't clear, though I assume it probably is because you both commented on #9599 and @Infinil had already said they were going to create a PR and created #11476 before you did. It would make sense to let @Infinil drive their PR to completion rather than requesting a review of your PR in order to merge yours before @Infinil's which already got an initial review.

@Infinil I'd suggest in the future to kindly comment (i.e. in your initial message) that you had already created a PR that does the same thing. You vaguely say so, but you don't specify where it is. I see @SwagatSBhuyan could have looked on #9599 to see the link, but sharing it here and not burying what "it is the same thing" will make it clearer to everyone that there is another PR which may also be older (it's fine if it wasn't and you accidentally ended up implementing the same thing (I've done that myself)).

Regarding this PR @SwagatSBhuyan, it does look like #11476 handles module.path is None the same way as the pre-existing code while this one does not. I'm not sure why module.path would be None and it looks like the CI is green on both of your PRs, but it's something to call out because you're changing the behavior (possibly in an OK way). My guess is a dynamic or eval'ed module may have no path 🤔. Also, a comment on your PR title. I would suggest in the future you make sure the PR title describes the change you are making not just what you're fixing (since the issue title is not actually the problem, and you are not making it not skip the vendor directory). #11476 is a pretty good example of what it could have been, because both of your changes are printing what modules are skipped which allows a user to understand why vendor would be ignored.

Oh yes, I saw the earlier PRs. Sorry for the inconvenience caused, maybe I should've just made the changes in the pre-existing PR instead of making another one. I also did nearly the same mistake before in another issue PR as I was new (I still am) to contributing in GitHub. I would keep that in mind next time. I will be closing this PR now, thanks.

No prob! Just trying to help not duplicate work for anyone. Feel free to use GitHub's suggestion feature for small suggestions and you can branch off another person's fork and create a PR to their PR branch in order to allow them to merge your follow up into theirs and that will automatically update the original PR with your larger suggestions 🙂.

That way we can work together instead of duplicating 😉