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.

Sign up for GitHub

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

Jump to bottom

Deprecate tests used as filters #32361

Merged
merged 18 commits into from Nov 27, 2017
Merged

Deprecate tests used as filters #32361

merged 18 commits into from Nov 27, 2017

Conversation

sivel
Copy link
Member

@sivel sivel commented Oct 30, 2017 •
edited

SUMMARY

This PR adds a deprecation warning for using jinja tests as filters for eventual removal.

See ansible/proposals#83

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

tests/filters

ANSIBLE VERSION
devel
ADDITIONAL INFORMATION

Things to be done:

  • Update docs to use proper test syntax instead of filter syntax
  • Update tests to use proper test syntax instead of filter syntax

@sivel sivel mentioned this pull request Oct 30, 2017
Closed
@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.5 This issue/PR affects Ansible v2.5 feature_pull_request needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Oct 30, 2017
@bcoca
Copy link
Member

bcoca commented Oct 30, 2017

I would not warn ... if we want to remove this make it a deprecation, not a warning.

@ansibot
Copy link
Contributor

ansibot commented Oct 30, 2017

cc @dharmabumstead
click here for bot help

@sivel
Copy link
Member Author

sivel commented Oct 30, 2017

I would not warn ... if we want to remove this make it a deprecation, not a warning.

Apologies, the title doesn't reflect functionality, we are using deprecations.

@sivel sivel changed the title [WIP] Warn on tests used as filters [WIP] Deprecate tests used as filters Oct 30, 2017
@sivel
Copy link
Member Author

sivel commented Oct 30, 2017 •
edited

I've taken this a step forward, and aliased some tests to names that work better grammatically when using the is syntax.

I've also added a deprecation warning for using the old test names.

@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Oct 30, 2017
@ansibot
Copy link
Contributor

ansibot commented Oct 30, 2017

The test ansible-test sanity --test rstcheck [?] failed with the following error:

docs/docsite/rst/playbooks_tests.rst:20:0: Unknown target name: "test syntax<http://jinja.pocoo.org/docs/dev/templates/#tests>".

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Nov 1, 2017

The test ansible-test sanity --test rstcheck [?] failed with the following error:

docs/docsite/rst/playbooks_tests.rst:20:0: Unknown target name: "test syntax<http://jinja.pocoo.org/docs/dev/templates/#tests>".

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Nov 1, 2017
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Nov 8, 2017
dharmabumstead
dharmabumstead approved these changes Nov 13, 2017
View reviewed changes
Copy link
Contributor

@dharmabumstead dharmabumstead left a comment

Choose a reason for hiding this comment

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

Thanks @sivel

@sivel sivel mentioned this pull request Nov 14, 2017
Closed
@gundalow gundalow mentioned this pull request Nov 14, 2017
Closed
@ansibot
Copy link
Contributor

ansibot commented Nov 15, 2017

cc @kedarX @mikewiebe @privateip @rahushen @rcarrillocruz @trishnaguha
click here for bot help

@ansibot ansibot added cloud cloudstack net_tools Net-tools category networking Network category nxos Cisco NXOS community selinux test This PR relates to tests. labels Nov 15, 2017
@ansibot
Copy link
Contributor

ansibot commented Nov 27, 2017

cc @mpdehaan
click here for bot help

@sivel
Copy link
Member Author

sivel commented Nov 27, 2017

rebuild_merge

@astorije
Copy link
Contributor

Off-topic but seeing @ansibot's message above: is there a way to opt-out from @ansibot? I haven't participated to this project in well over a year and get mentions I'd like to silence (without muting the whole repo). I looked at the help link it provides and no luck.
Thanks!

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Nov 27, 2017
@sivel
Copy link
Member Author

sivel commented Nov 27, 2017

rebuild_merge

One more automated try

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 27, 2017
@ansibot ansibot merged commit 4fe0844 into ansible:devel Nov 27, 2017
@sivel sivel mentioned this pull request Nov 28, 2017
Merged
@willthames willthames mentioned this pull request Dec 7, 2017
Closed
@sivel sivel mentioned this pull request Dec 7, 2017
Merged
@sivel sivel mentioned this pull request Dec 14, 2017
Merged
@astorije astorije mentioned this pull request Dec 26, 2017
Closed
@ansibot ansibot added docs This issue/PR relates to or includes documentation. feature This issue/PR relates to a feature request. and removed docs_pull_request labels Mar 4, 2018
@felixfontein felixfontein mentioned this pull request Apr 7, 2018
Merged
@dagwieers dagwieers added the cisco Cisco technologies label Feb 23, 2019
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dharmabumstead dharmabumstead dharmabumstead approved these changes

@abadger abadger Awaiting requested review from abadger

@bcoca bcoca Awaiting requested review from bcoca

@thaumos thaumos Awaiting requested review from thaumos

@gundalow gundalow Awaiting requested review from gundalow

@jborean93 jborean93 Awaiting requested review from jborean93

Assignees
No one assigned
Labels
affects_2.5 This issue/PR affects Ansible v2.5 cisco Cisco technologies cloud cloudstack core_review In order to be merged, this PR must follow the core review workflow. docker docs This issue/PR relates to or includes documentation. f5 feature This issue/PR relates to a feature request. module This issue/PR relates to a module. net_tools Net-tools category networking Network category nxos Cisco NXOS community plugins/test selinux support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. support:network This issue/PR relates to code supported by the Ansible Network Team. test This PR relates to tests. windows Windows community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@sivel @bcoca @ansibot @dharmabumstead @abadger @astorije @dagwieers