Install & run sh files Shellcheck warnings fix #46
Conversation
There was a problem hiding this comment.
Hey, thanks for the PR!
I'm not sure if I want to touch this working bash script at all, it's super fragile and we might be breaking the use-case of someone unknowingly while having good intentions as fixing some lints.
If we are going to go ahead with this, it is required that we test this very well and understand each lint change and how it effects the script...
You watched me suffer on livestream yesterday... I don't want that to happen again :D
|
I know, I know. You can't imagine how much I fought with scripts. But at some point you really need to base a field. Shellchek really helps with that. Of course, I'll leave the decision up to you for now. I wanted to help. Also, if you merge this PR request, I created a second PR (#47). Here, we can save time and effort by adding the automatic check to the action. Note: I don't have enough familiarity with the project to test it. It would require extensive time. So I'm hesitant to take on this task directly. But I'll take a look. I'm one comment away. I'd like to do my best. |
|
I might check this out before a new release, which is unplanned for now. A bit of an investigation for these changes would be always appreciated. |
|
Resolved the conflict by rebasing the main branch. |
Greetings @orhun,
I follow you for your projects and works. Good work!
I want to make a small contribution to this project. I provided a short review for warning messages using Shellcheck. However, I am not familiar with the project and spent a short time. I could definitely be wrong. I am creating a PR for testing. I will look at the content that does not progress properly in actions. Also, if you can guide me with explanations, I would like to help more.
Good day, greetings from the broadcast!
Additional Note: If you deem it appropriate, you can automatically check the contents of the bash scripts with shellcheck during PR checks, this can be useful when there are syntax-based errors or warnings.