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

Fix generate.sh from bad trap for ERR signal by using bash instead of sh #9243

Merged
merged 1 commit into from
Jan 20, 2022

Conversation

noahkawasakigoogle
Copy link
Contributor

@noahkawasakigoogle noahkawasakigoogle commented Jan 1, 2022

What

Reports of some users seeing bad trap when running airbyte-integrations/connector-templates/generator/generate.sh because ERR does not exist in POSIX.

#5586

How

Use bash instead. I noticed a lot of the shell scripts in the repo use #!/usr/bin/env bash but a lot of the connectors use sh in the acceptance-test-docker.sh scripts. I was confused at first but it seems like the reason for this is that the generator docker image takes the last created connector and creates a template based off of that. So unless there was an intentional reason for using sh I think changing everything to bash now would solve this for the future until a linter rule is created to require bash.

Recommended reading order

  1. x.java
  2. y.python

🚨 User Impact 🚨

No user impact

Pre-merge Checklist


This change is Reviewable

@CLAassistant
Copy link

CLAassistant commented Jan 1, 2022

CLA assistant check
All committers have signed the CLA.

@noahkawasakigoogle noahkawasakigoogle changed the title Fix generate.sh from bad trap for ERR signal in by using bash instead of sh Fix generate.sh from bad trap for ERR signal by using bash instead of sh Jan 1, 2022
@noahkawasakigoogle
Copy link
Contributor Author

FYI I need to get an approval from my employer (Google) on signing the CLA first. They've got some processes around open source contributions

@marcosmarxm marcosmarxm self-assigned this Jan 3, 2022
@noahkawasakigoogle noahkawasakigoogle marked this pull request as draft January 10, 2022 00:10
@noahkawasakigoogle noahkawasakigoogle marked this pull request as ready for review January 19, 2022 15:25
@noahkawasakigoogle
Copy link
Contributor Author

Okay I got approval to sign the CLA, I can contribute to MIT licensed files, which this is. So should be okay to merge this one

@tuliren tuliren merged commit 97eeb2f into airbytehq:master Jan 20, 2022
@noahkawasakigoogle noahkawasakigoogle deleted the noah/fix-generate-sh branch January 20, 2022 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants