Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Update the action and inherit ansible-lint arguments #7

Merged
merged 7 commits into from
Oct 28, 2019

Conversation

CermakM
Copy link

@CermakM CermakM commented Oct 10, 2019

  • the action now takes a limited set of arguments and passes them to ansible-lint (see the README
  • created action.yaml
  • updated documentation

An example run can be found here.

Marek Cermak added 4 commits October 10, 2019 20:56
- a relevant set of arguments is inherited (i.e., arguments like --help
or -L are filtered out)

- write action.yml file

Signed-off-by: Marek Cermak <macermak@redhat.com>

new file:   action.yml
modified:   entrypoint.sh
Signed-off-by: Marek Cermak <macermak@redhat.com>

modified:   entrypoint.sh
This prevents potential misunderstendings since ansible-lint does not
implicitly recorse directories, which might not be the expected
behaviour with default '.' value.

Signed-off-by: Marek Cermak <macermak@redhat.com>

modified:   action.yml
- be more explicit in the `targets` description

Signed-off-by: Marek Cermak <macermak@redhat.com>

modified:   action.yml
modified:   readme.md
@CermakM
Copy link
Author

CermakM commented Oct 10, 2019

Fixes: #6

@stoe stoe added the enhancement New feature or request label Oct 11, 2019
@stoe
Copy link
Contributor

stoe commented Oct 11, 2019

Thanks for the contribution @CermakM.
I will take a look this weekend.

@stoe
Copy link
Contributor

stoe commented Oct 14, 2019

Doing some more test @CermakM, but so far it looks promising.

@CermakM
Copy link
Author

CermakM commented Oct 23, 2019

@stoe any update on this?

entrypoint.sh Outdated
@@ -1,30 +1,98 @@
#! /usr/bin/env bash

set -eo pipefail
set -euo pipefail
Copy link
Member

Choose a reason for hiding this comment

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

Let's do even better:

Suggested change
set -euo pipefail
set -Eeuo pipefail

entrypoint.sh Outdated
exit 1
fi
local opts=$(parse_args "$@" || exit 1)
if [ "$opts" = "" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

This clause is totally unnecessary.

@webknjaz
Copy link
Member

@CermakM That's quite a lot of shell-script args parsing boilerplate. Since we use docker, it's easy to rely on some easier to understand language.

The original idea was to rewrite it in Python. SH was just a quick hack.
Any chance you could do this? It should be very easy to wrap it with Click.

@stoe
Copy link
Contributor

stoe commented Oct 26, 2019

@stoe any update on this?

Sorry for the delay, I overcommitted on my initial promise.

The original idea was to rewrite it in Python. SH was just a quick hack.
Any chance you could do this?

That would be ✨ amazing.
Not sure I can help with this right now.

@webknjaz
Copy link
Member

@CermakM if you're not willing to rewrite it into Python, I can merge it for now and do it myself once I have time. If you need any directions, feel free to reach out to me and we could even meet @ TPB-C.

Signed-off-by: Marek Cermak <macermak@redhat.com>

modified:   entrypoint.sh
@CermakM
Copy link
Author

CermakM commented Oct 27, 2019

@webknjaz Ye, rewriting it to Python would be great, we could add some unit tests to it then as well. If it is possible, however, I would leave it to a separate PR (it would take some time to re-write it to Python and test it all over again). So if we could merge this one, it'd be great.

PS: Removed the don't-know-how-it-got-there if clause. Thanks for the comment.

@webknjaz webknjaz merged commit 4d11c71 into ansible:master Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants