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

Feature/allow multilines build scripts #282

Merged
merged 2 commits into from
Feb 23, 2021
Merged

Feature/allow multilines build scripts #282

merged 2 commits into from
Feb 23, 2021

Conversation

fpalluel
Copy link
Contributor

Description

See #281

@npalm
Copy link
Collaborator

npalm commented Jan 28, 2021

Will check asap

@npalm
Copy link
Collaborator

npalm commented Jan 28, 2021

Did a quick check on the default example. Instance is created but agent does NOT start. In complaints about the pre en post build script. Seem the multi line proposal is not working. I had not the time to dig in further. Hope you have an idea how to fix it.

@fpalluel
Copy link
Contributor Author

Hello,
Sorry, I was so focused with the multiline case, that I made a mistake with the single case !

In the example:
runners_post_build_script = "echo 'single line'"

should be:
runners_post_build_script = "\"echo 'single line'\""

which is, admittedly, ugly, but I cannot see any other mean to make both single and multilines cases work :-(
If you agree with this syntax, I'll fix the PR at once.

@npalm
Copy link
Collaborator

npalm commented Feb 2, 2021

Sorry for the late response, will check later today.

@npalm
Copy link
Collaborator

npalm commented Feb 2, 2021

Adding ref to gitlab docs: https://docs.gitlab.com/runner/configuration/advanced-configuration.html

@npalm
Copy link
Collaborator

npalm commented Feb 2, 2021

Hello,
Sorry, I was so focused with the multiline case, that I made a mistake with the single case !

In the example:
runners_post_build_script = "echo 'single line'"

should be:
runners_post_build_script = "\"echo 'single line'\""

which is, admittedly, ugly, but I cannot see any other mean to make both single and multilines cases work :-(
If you agree with this syntax, I'll fix the PR at once.

LGTM

@npalm
Copy link
Collaborator

npalm commented Feb 11, 2021

@fpalluel did you had time to have a look on the PR?

@fpalluel
Copy link
Contributor Author

@fpalluel did you had time to have a look on the PR?

Hello, just did it, sorry for the delay ;-)

npalm
npalm requested changes Feb 14, 2021
View reviewed changes
Copy link
Collaborator

@npalm npalm left a comment

Choose a reason for hiding this comment

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

All good, please can you rebase the PR?

@fpalluel fpalluel force-pushed the feature/allow-multilines-build-scripts branch 3 times, most recently from 79fbbf6 to c318510 Compare February 14, 2021 16:38
@fpalluel
Copy link
Contributor Author

Locally on MacOs I run :

  • Terraform v0.14.5
  • Terraform-docs version v0.11.0
  • TFLint version 0.22.0

And the pre-commit hook execution leads to differences in generated Terraform docs

@npalm
Copy link
Collaborator

npalm commented Feb 15, 2021

Locally on MacOs I run :

  • Terraform v0.14.5
  • Terraform-docs version v0.11.0
  • TFLint version 0.22.0

And the pre-commit hook execution leads to differences in generated Terraform docs

Seems missing in the docs, but the moduel is still on terraform 13. I use tfenv to swithc terraform version. Can you try the pre-commit hooks with terraform 13. In case the troubles remain, I can do a local merge

@fpalluel fpalluel force-pushed the feature/allow-multilines-build-scripts branch from c318510 to 6697ecf Compare February 15, 2021 08:27
@fpalluel
Copy link
Contributor Author

Hello,
I just tried with Terraform 13.6, but the issue seems to be with Terraform-docs 0.11.0 (see releases). The latest release brings some changes in the generated content, adding some sections.

So I switched back to Terraform-docs 0.10.1, but there are still changes in the generated content (headers formatting for instance)...
I don't know why, but I cannot reproduce the generated content formatting of the develop branch.

@npalm npalm merged commit 7000c07 into cattle-ops:develop Feb 23, 2021
github-actions bot pushed a commit that referenced this pull request Feb 28, 2021
@semantic-release-bot
chore(release): 4.23.0 [skip ci]
2ab51ab 
## [4.23.0](4.22.0...4.23.0) (2021-02-28)

### Features

* additional config parameter asg_delete_timeout to configure the timeout when trying to delete the ASG ([#305](#305)) ([f60c9d5](f60c9d5))
* allow multilines build scripts ([#282](#282)) ([7000c07](7000c07)), closes [#250](#250)

### Bug Fixes

* autoscaling configuraton ([#301](#301)) ([6b35a10](6b35a10))
* respect create_cache_bucket variable and avoid concurrent changes to cache bucket ([#296](#296)) ([c3629f6](c3629f6))
@semantic-releaser
Copy link
Contributor

🎉 This PR is included in version 4.23.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@npalm npalm npalm approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fpalluel @npalm