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

Feat [Shell]: Add an attribute which suppress the source appending #304

Merged
merged 2 commits into from
Oct 7, 2021

Conversation

jplayout
Copy link
Contributor

@jplayout jplayout commented Oct 7, 2021

Add an attribute which suppress the source appending

Fix #288

I added an boolean attribute named suppressSource which removes the source added to the command

@dduportal
Copy link
Contributor

Documentation PR (from @jplayout also): updatecli/website#140

@dduportal dduportal added condition Modify condition resource hacktoberfest-accepted resource-shell Resource of kind Shell labels Oct 7, 2021
@dduportal dduportal enabled auto-merge (squash) October 7, 2021 15:11
@dduportal dduportal added the enhancement New feature or request label Oct 7, 2021
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

LGMT 👍

@dduportal dduportal requested a review from olblak October 7, 2021 15:12
@olblak
Copy link
Member

olblak commented Oct 7, 2021

Thank you a lot @jplayout for the PR.
I am facing the same issue on other "resources" such as "aws/ami" and I am wondering if it wouldn't be better to have something generic at the condition and at target level such as (not at the spec level like you are suggesting).

I am thinking of something like:

conditions:
    condition:
        disableSourceInput: true (default to false)
        kind: shell
        spec:
            command: xxx

And we could just have a condition that test if disableSourceInput is set to true then we set the source to an empty string
here

@dduportal
Copy link
Contributor

@olblak WDYT about merging this PR (eventually adapting the attribute name) to target a delivery for the 0.8.2 milestone, and then work on the 0.9.0 with a change like yours? Because an issue has to be written to think about the meaning of "source input" with your thoughts at least on the issue?

The reason is because we tagged the issue #288 to be first good issue, and we are exteding the scope now that the PR is open... ;)

@dduportal
Copy link
Contributor

Thats weird: I never clicked the merge button, neither did I enable the auto-merge 😕
Sorry for the inconvenience @olblak : don't hesitate to revert and a PR would be re-opened then

@olblak
Copy link
Member

olblak commented Oct 7, 2021

weird that the pr got merged anyway you made a good suggestion, I need to think a little bit about my suggestion and both are not incompatible. So let's move forward. I would just target a new minor version instead of a patch version

@jplayout jplayout deleted the gh-288 branch October 12, 2021 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
condition Modify condition resource enhancement New feature or request hacktoberfest-accepted resource-shell Resource of kind Shell
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request [Resource "shell"] Allow disabling adding the source as line argument
3 participants