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

FEDX-2722: Update the ignore field to ignore everything, not just specific checks #148

Conversation

matthewnitschke-wk
Copy link
Contributor

@matthewnitschke-wk matthewnitschke-wk commented Mar 6, 2025

FEDX-2722

Issue Status

Motivation

Currently, the ignore field only applies to some rules in the dependency_validator execution process

If you're explicitly ignoring a dependency, it doesn't make sense that some rules can still fail. As such, this PR changes the expectation of evaluation, where when ignore exists for a package, its full ignored from dep evaluation

Tip

Note, this PR's base is v4 and not master

We need to backpatch the v4 version with this change, and this will be manually released once merged into v4. We will create a similar PR in master so the functionality between these branches doesn't change

Testing/QA Instructions

  • Consume in a package that fails the "The following packages contain executables, and are only used outside of lib/. These should be downgraded to dev_dependencies:" rule, and ensure that adding ignore indeed ignores it

@matthewnitschke-wk matthewnitschke-wk changed the title Support the ignore field for packages that are ignored, contain execu… Ignoring a package name should ignore it everywhere Mar 6, 2025
@matthewnitschke-wk matthewnitschke-wk changed the title Ignoring a package name should ignore it everywhere Update the ignore field to ignore everything, not just specific checks Mar 6, 2025
@bender-wk bender-wk changed the title Update the ignore field to ignore everything, not just specific checks FEDX-2722: Update the ignore field to ignore everything, not just specific checks Mar 6, 2025
@matthewnitschke-wk matthewnitschke-wk marked this pull request as ready for review March 6, 2025 22:24
@matthewnitschke-wk matthewnitschke-wk requested a review from a team as a code owner March 6, 2025 22:24
Copy link

@danielszopa-wk danielszopa-wk left a comment

Choose a reason for hiding this comment

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

+1 I can confirm this fixes the issue this xbrl-module PR was hitting.

@alanknight-wk
Copy link

@Workiva/release-management-p

Copy link
Contributor

@rmconsole-wf rmconsole-wf left a comment

Choose a reason for hiding this comment

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

+1 from RM

@btr-rmconsole-4 btr-rmconsole-4 bot merged commit f47c8d7 into v4 Mar 6, 2025
12 checks passed
@btr-rmconsole-4 btr-rmconsole-4 bot deleted the support_ignore_for_packges_with_executables_only_used_outside_lib branch March 6, 2025 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants