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

[tooling/impi] Evaluate packages of current module only #9946

Merged
merged 1 commit into from
May 11, 2022

Conversation

djaglowski
Copy link
Member

Related to #9888

Currently causing issues with excluding a module on #9922

@bogdandrutu
Copy link
Member

Just temporary disable impi everywhere, or another option is to have a Makefile with all the rules do nothing (test, impi, etc.)

@djaglowski
Copy link
Member Author

I actually think this is the correct change. Some more details in #9888, but basically the problem is that impi is currently overlapping coverage between nested modules. This means there is no way to exclude a nested module because make impi at the root module will cover it anyways.

I believe this change results in no loss of coverage in our CI though, as we still run make impi in each module as part of make golint. The only way in which this changes coverage is if someone runs make impi at the root module and expects it to cover all modules. For that case, I have proposed in #9888 that there should be a corresponding "all-impi" (name tbd) target that runs impi for each module.

@djaglowski djaglowski added the Skip Changelog PRs that do not require a CHANGELOG.md entry label May 11, 2022
@djaglowski djaglowski marked this pull request as ready for review May 11, 2022 13:27
@djaglowski djaglowski requested review from a team and codeboten May 11, 2022 13:27
@djaglowski
Copy link
Member Author

@bogdandrutu, do you mind taking another look at this? If you are ok with this change based on my comment above, this will allow #9922 to move forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants