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

Add shellcheck workflow, and tune up docker/entrypoint.sh to become clean #71

Merged
merged 1 commit into from
Nov 28, 2022

Conversation

yarikoptic
Copy link
Member

Primarily constituted addressing two following warnings from shellcheck:

In entrypoint.sh line 55:
if [ ! -z "${INPUT_UPDATE_LOOKUP}" ]; then
	 ^-- SC2236 (style): Use -n instead of ! -z.

In entrypoint.sh line 60:
printf "Run zenodo: ${RUN_ZENODO}\n"
	   ^---------------------------^ SC2059 (info): Don't use variables in the printf format string. Use printf '..%s..' "$foo".

and for consistency I replaced all other uses of 'printf' with simple 'echo'.

Note: tests/*.sh were not treated and neither added to shellcheck workflow. Wanted first to assess if such changes are desired, can do similar act for tests if agreed to proceed.

…lean

Primarily constituted addressing two following warnings from shellcheck:

	In entrypoint.sh line 55:
	if [ ! -z "${INPUT_UPDATE_LOOKUP}" ]; then
		 ^-- SC2236 (style): Use -n instead of ! -z.

	In entrypoint.sh line 60:
	printf "Run zenodo: ${RUN_ZENODO}\n"
		   ^---------------------------^ SC2059 (info): Don't use variables in the printf format string. Use printf '..%s..' "$foo".

and for consistency I replaced all other uses of 'printf' with simple
'echo'.
@yarikoptic yarikoptic requested a review from vsoch November 3, 2022 12:28
@yarikoptic
Copy link
Member Author

@vsoch ping -- any blessing or objections?

@vsoch
Copy link
Collaborator

vsoch commented Nov 17, 2022

No objections! But if you are bringing in different linting, I'd recommend pre-commit https://github.com/koalaman/shellcheck-precommit and then add black/ isort and others there. I've been using it for most of my python projects and it's magical.

Not a hard requirement, just a suggestion for a nice developer experience. ✨

@yarikoptic
Copy link
Member Author

yeah, we also use pre-commit in dandi-cli. I love and not it ;) someone might at some point in the future. Let's proceed with this for now as is.

@yarikoptic yarikoptic added the internal Changes only affect the internal API label Nov 28, 2022
@yarikoptic yarikoptic merged commit c622750 into master Nov 28, 2022
@yarikoptic yarikoptic deleted the enh-shellcheck branch November 28, 2022 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Changes only affect the internal API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants