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

bash should be added as a dependency, for now #41

Closed
keevan opened this issue Apr 2, 2022 · 3 comments · Fixed by #42
Closed

bash should be added as a dependency, for now #41

keevan opened this issue Apr 2, 2022 · 3 comments · Fixed by #42

Comments

@keevan
Copy link
Contributor

keevan commented Apr 2, 2022

https://github.com/ChaoticWeg/discord.sh/blob/2f2c0f4c14b48474a6a360eb401c040016cfbedb/README.md?plain=1#L32-L36

I've tried running this on sh and there were a few bash specific things that stopped this from getting very far, namely

discord.sh: line 6: shopt: not found
discord.sh: line 7: shopt: not found
discord.sh: line 31: syntax error: bad substitution

There's probably many more that it hasn't reached yet but it might be easier to set the requirements to include bash.

keevan added a commit to keevan/discord.sh that referenced this issue Apr 2, 2022
@fieu
Copy link
Owner

fieu commented Apr 3, 2022

I've tried running this on sh and there were a few bash specific things that stopped this from getting very far...

This project has never mentioned supporting sh so I don't know why you are trying it.

Refer to #42 for request of changes regarding your contribution.

@fieu fieu closed this as completed Apr 3, 2022
keevan added a commit to keevan/discord.sh that referenced this issue Apr 3, 2022
keevan added a commit to keevan/discord.sh that referenced this issue Apr 3, 2022
@keevan
Copy link
Contributor Author

keevan commented Apr 3, 2022

This project has never mentioned supporting sh so I don't know why you are trying it.

So the reason I stumbled upon this was because I saw .sh in the title of the project (I haven't checked it's origins), and also because I tested it locally (on a machine with bash), tested some options, and looked at the requirements/dependencies. I then wanted to add this to an alpine image (due to it's compact size). After adding it, going through the motions, and finally executing it, it didn't work the way I thought it would on sh.

Yes, there is no mention of support for sh, but I thought I'd save time potentially for others like me by listing it explicitly in the dependencies.

@fieu
Copy link
Owner

fieu commented Apr 4, 2022

You're right and that's my bad on the perspective I had when writing the docs. I've gone ahead and accepted your contribution in adding the specific mention of bash in the requirements list.

Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants