Skip to content
This repository has been archived by the owner on Aug 4, 2023. It is now read-only.

Add ShellCheck to pre-commit config #718

Merged
merged 8 commits into from
Sep 20, 2022
Merged

Add ShellCheck to pre-commit config #718

merged 8 commits into from
Sep 20, 2022

Conversation

MustkimKhatik
Copy link
Contributor

@MustkimKhatik MustkimKhatik commented Sep 8, 2022 •
edited by dhruvkb
Loading

Related to WordPress/openverse#289

adding shellcheck to openverse-catalog repo

@MustkimKhatik
Update .pre-commit-config.yaml
a058c60 
adding shellcheck to openverse-catalog
@MustkimKhatik MustkimKhatik requested a review from a team as a code owner September 8, 2022 14:10
@openverse-bot openverse-bot added the 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work label Sep 8, 2022
dhruvkb
dhruvkb suggested changes Sep 8, 2022
View reviewed changes
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@dhruvkb dhruvkb added 🟩 priority: low Low priority and doesn't need to be rushed 🤖 aspect: dx Concerns developers' experience with the codebase 🧰 goal: internal improvement Improvement that benefits maintainers, not users and removed 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work labels Sep 8, 2022
@MustkimKhatik @dhruvkb
Update .pre-commit-config.yaml
2fd0a38 
Co-authored-by: Dhruv Bhanushali <dhruv_b@live.com>
@MustkimKhatik
Copy link
Contributor Author

Yeah I did that change

@dhruvkb
Copy link
Member

dhruvkb commented Sep 8, 2022

@MustkimKhatik ShellCheck is now running as expected, and flagging a number of warnings and errors. Could you take a look at the ShellCheck output and implement the changes it recommends?

@MustkimKhatik
Copy link
Contributor Author

Yeah, I'll do them!

@krysal
Copy link
Member

krysal commented Sep 15, 2022 •
edited
Loading

@MustkimKhatik are you still able to take on the ShellCheck recommendations to fix the "CI + CD / Lint" check?

@MustkimKhatik
Copy link
Contributor Author

Yep, I'm on them.

@MustkimKhatik
Merge branch 'WordPress:main' into patch-1
25b20dc
@MustkimKhatik
Update entrypoint.sh
2da13fe 
Syntax correction in ```entrypoint.sh```
@MustkimKhatik
Update dag-sync.sh
11777bb 
Syntax correction in ```dag-sync.sh```
@MustkimKhatik
Update prettify.sh
08a1b8d 
Syntax correction in ```prettify.sh```
@MustkimKhatik
Copy link
Contributor Author

Hey @krysal, current in dag-sync.sh, it was unused variable and to resolve it I used echo. there was a option to export too. I will appreciate if you tell me what will be more appropriate here to use?

@krysal krysal requested a review from dhruvkb September 16, 2022 14:40
dhruvkb
dhruvkb approved these changes Sep 18, 2022
View reviewed changes
Copy link
Member

@dhruvkb dhruvkb left a comment

Choose a reason for hiding this comment

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

Apart from one change, this looks good to me. Thanks @MustkimKhatik!

dag-sync.sh Outdated Show resolved Hide resolved
@MustkimKhatik
Copy link
Contributor Author

Apart from one change, this looks good to me. Thanks @MustkimKhatik!

Sure, I'll do that change.

@MustkimKhatik
Update dag-sync.sh
8e71043 
some syntax correction in dag-sync
@AetherUnbound AetherUnbound changed the title Update .pre-commit-config.yaml Add spellcheck to pre-commit config Sep 20, 2022
obulat
obulat approved these changes Sep 20, 2022
View reviewed changes
Copy link
Contributor

@obulat obulat left a comment

Choose a reason for hiding this comment

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

This is such a useful addition, it feels like magic! Thank you for your contribution, @MustkimKhatik, and welcome to Openverse!

@MustkimKhatik
Copy link
Contributor Author

This is such a useful addition, it feels like magic! Thank you for your contribution, @MustkimKhatik, and welcome to Openverse!

You're welcome and that's my pleasure to work with openverse!

AetherUnbound
AetherUnbound approved these changes Sep 20, 2022
View reviewed changes
Copy link
Contributor

@AetherUnbound AetherUnbound left a comment

Choose a reason for hiding this comment

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

Thank you for the addition, the changes look great!

@dhruvkb dhruvkb merged commit 28bfd16 into WordPress:main Sep 20, 2022
@dhruvkb dhruvkb changed the title Add spellcheck to pre-commit config Add ShellCheck to pre-commit config Sep 20, 2022
@dhruvkb dhruvkb mentioned this pull request Sep 25, 2022
Closed
5 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dhruvkb dhruvkb dhruvkb approved these changes

@obulat obulat obulat approved these changes

@AetherUnbound AetherUnbound AetherUnbound approved these changes

@stacimc stacimc Awaiting requested review from stacimc stacimc is a code owner automatically assigned from WordPress/openverse-catalog

Assignees
No one assigned
Labels
🤖 aspect: dx Concerns developers' experience with the codebase 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟩 priority: low Low priority and doesn't need to be rushed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@MustkimKhatik @dhruvkb @krysal @AetherUnbound @obulat @openverse-bot