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

Remove progressive mode #3350

Merged
merged 2 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .ansible-lint
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,6 @@ warn_list:
# Offline mode disables installation of requirements.yml and schema refreshing
offline: true

# Return success if number of violations compared with previous git
# commit has not increased. This feature works only in git
# repositories.
progressive: false

# Define required Ansible's variables to satisfy syntax check
extra_vars:
foo: bar
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 795
PYTEST_REQPASS: 794
steps:
- name: Activate WSL1
if: "contains(matrix.shell, 'wsl')"
Expand Down
4 changes: 1 addition & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ pip-wheel-metadata
# mypy
.mypy_cache

# .cache is used by progressive mode
.cache

# Generated by setuptools-scm
src/ansiblelint/_version.py

Expand All @@ -59,6 +56,7 @@ test/fixtures/formatting-before/
examples/playbooks/vars/strings.transformed.yml

# other
.cache
.DS_Store
.vscode
.idea
Expand Down
30 changes: 12 additions & 18 deletions docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,18 @@ Ansible-lint creates a new cache on the next invocation.
You should add the `.cache` folder to the `.gitignore` file in your git
repositories.

## Using progressive mode (deprecated)

!!! warning

This feature is deprecated and will be removed in the next major release.
We encourage you to use [ignore file](configuring.md#ignoring-rules-for-entire-files)
instead.

For easier adoption, Ansible-lint can alert for rule violations that occur since
the last commit. This allows new code to be merged without any rule violations
while allowing content developers to address historical violations at a
different pace.

The `--progressive` option runs Ansible-lint twice if rule violations exist in
your content. The second run is performed against a temporary git working copy
that contains the last commit. Rule violations that exist in the last commit are
ignored and Ansible-lint displays only the violations that exist in the new
commit.
# Gradual adoption

For an easier gradual adoption, adopters should consider [ignore
file][configuring.md#ignoring-rules-for-entire-files] feature. This allows the
quick introduction of a linter pipeline for preventing addition of new
violations, while known violations are ignored. Some people can work on
addressing these historical violations while others may continue to work on
other maintenance tasks.

The deprecated `--progressive` mode was removed in v6.16.0 as it added code
complexity and performance overhead. It also presented several corner cases
where it failed to work as expected and caused false negatives.

## Linting playbooks and roles

Expand Down
37 changes: 0 additions & 37 deletions src/ansiblelint/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,6 @@ def main(argv: list[str] | None = None) -> int: # noqa: C901
_logger.debug("Options: %s", options)
_logger.debug(os.getcwd())

if options.progressive:
_logger.warning(
"Progressive mode is deprecated and will be removed in next major version, use ignore files instead: https://ansible-lint.readthedocs.io/configuring/#ignoring-rules-for-entire-files",
)

if not options.offline:
# pylint: disable=import-outside-toplevel
from ansiblelint.schemas import refresh_schemas
Expand Down Expand Up @@ -261,38 +256,6 @@ def main(argv: list[str] | None = None) -> int: # noqa: C901
_do_transform(result, options)

mark_as_success = True
if result.matches and options.progressive:
mark_as_success = False
_logger.info(
"Matches found, running again on previous revision in order to detect regressions",
)
with _previous_revision():
_logger.debug("Options: %s", options)
_logger.debug(os.getcwd())
old_result = _get_matches(rules, options)
# remove old matches from current list
matches_delta = list(set(result.matches) - set(old_result.matches))
if len(matches_delta) == 0:
_logger.warning(
"Total violations not increased since previous "
"commit, will mark result as success. (%s -> %s)",
len(old_result.matches),
len(matches_delta),
)
mark_as_success = True

ignored = 0
for match in result.matches:
# if match is not new, mark is as ignored
if match not in matches_delta:
match.ignored = True
ignored += 1
if ignored:
_logger.warning(
"Marked %s previously known violation(s) as ignored due to"
" progressive mode.",
ignored,
)

if options.strict and result.matches:
mark_as_success = False
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def report_outcome(self, result: LintResult, mark_as_success: bool = False) -> i
"because 'yaml' is in 'skip_list'.",
)

if mark_as_success and summary.failures and not self.options.progressive:
if mark_as_success and summary.failures:
mark_as_success = False

if not self.options.quiet:
Expand Down
10 changes: 0 additions & 10 deletions src/ansiblelint/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,15 +293,6 @@ def get_cli_parser() -> argparse.ArgumentParser:
action="store_true",
help="parseable output, same as '-f pep8'",
)
parser.add_argument(
"--progressive",
dest="progressive",
default=False,
action="store_true",
help="Return success if number of violations compared with "
"previous git commit has not increased. This feature works "
"only in git repositories.",
)
parser.add_argument(
"--project-dir",
dest="project_dir",
Expand Down Expand Up @@ -472,7 +463,6 @@ def merge_config(file_config: dict[Any, Any], cli_config: Options) -> Options:
"quiet",
"strict",
"use_default_rules",
"progressive",
"offline",
)
# maps lists to their default config values
Expand Down
6 changes: 1 addition & 5 deletions src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,7 @@ class Options: # pylint: disable=too-many-instance-attributes,too-few-public-me
enable_list: list[str] = field(default_factory=list)
skip_action_validation: bool = True
strict: bool = False
rules: dict[
str,
Any,
] = field(
rules: dict[str, Any] = field(
default_factory=dict,
) # Placeholder to set and keep configurations for each rule.
profile: str | None = None
Expand All @@ -144,7 +141,6 @@ class Options: # pylint: disable=too-many-instance-attributes,too-few-public-me
config_file: str | None = None
generate_ignore: bool = False
rulesdir: list[Path] = field(default_factory=list)
progressive: bool = False
cache_dir_lock: FileLock | None = None
use_default_rules: bool = False
version: bool = False # display version command
Expand Down
9 changes: 2 additions & 7 deletions src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,13 +284,8 @@ def get(self, key: Any, default: Any = None) -> Any:

def _populate_content_cache_from_disk(self) -> None:
# Can raise UnicodeDecodeError
try:
self._content = self.path.expanduser().resolve().read_text(encoding="utf-8")
except FileNotFoundError as exc:
if vars(options).get("progressive"):
self._content = ""
else:
raise exc
self._content = self.path.expanduser().resolve().read_text(encoding="utf-8")

if self._original_content is None:
self._original_content = self._content

Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/schemas/__store__.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/inventory.json"
},
"meta": {
"etag": "45184c720bb76a0160b44a0fe1cb024075ae275b9a978fd547cc2c0a14e32ecc",
"etag": "449550c81ab905d3ac30b2672bf23a43cf1a71eef168ab08e8caee8c4a32bce6",
"url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/meta.json"
},
"meta-runtime": {
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/schemas/ansible-lint-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
},
"progressive": {
"default": false,
"title": "Progressive",
"title": "Progressive (removed feature)",
"type": "boolean"
},
"quiet": {
Expand Down
93 changes: 0 additions & 93 deletions test/test_progressive.py

This file was deleted.

2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ passenv =
REQUESTS_CA_BUNDLE # https proxies
SETUPTOOLS_SCM_DEBUG
SSL_CERT_FILE # https proxies
SSH_AUTH_SOCK # may be needed by git when running with progressive
SSH_AUTH_SOCK # may be needed by git
LANG
LC_*
# recreate = True
Expand Down