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 multiple lint errors (shellcheck) and make some refactoring #768

Merged
merged 21 commits into from
Mar 9, 2016
Merged

Fix multiple lint errors (shellcheck) and make some refactoring #768

merged 21 commits into from
Mar 9, 2016

Conversation

vutny
Copy link
Contributor

@vutny vutny commented Feb 28, 2016

Plus style and logic corrections, mostly for FreeBSD and OpenBSD.

# NAME: linkfile
# DESCRIPTION: Simple function to create symlinks. Overrides if asked. Accepts globs.
#----------------------------------------------------------------------------------------------------------------------
linkfile() {
Copy link
Member

Choose a reason for hiding this comment

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

Please prefix this with __. The only intent is to "tell" someone reading that it's an internal function, there's no other gain from that.

@s0undt3ch
Copy link
Member

The cd .... || exit lint complaints can probably be disabled.

Thanks for cleaning up the script.

You can see the lint information right? https://jenkins-testing.saltstack.com/job/bootstrap/job/pr/job/768/job/lint/1/violations/file/bootstrap-salt.sh/

@s0undt3ch
Copy link
Member

Dam, this Jenkins setup is not right(it's setting commit status here form another PR)....

@vutny
Copy link
Contributor Author

vutny commented Feb 28, 2016

Thanks @s0undt3ch
I saw that Jenkins status, looked suspicious for me too :-) I'll fix function name and other warnings.

@vutny
Copy link
Contributor Author

vutny commented Feb 29, 2016

@s0undt3ch I've fixed all lint warnings from previous build, but the last time it showed up something totally unrelated to my version (obviously wrong PR 666): https://jenkins-testing.saltstack.com/job/bootstrap/job/pr/job/666/job/lint/34/violations/file/bootstrap-salt.sh/

I guess this PR is OK for now. Could you please take a look?

@vutny
Copy link
Contributor Author

vutny commented Mar 1, 2016

@s0undt3ch Do you have some time to review this PR? Thanks!

@vutny vutny changed the title Fix multiple lint errors (shellcheck) Fix multiple lint errors (shellcheck) and make some refactoring Mar 9, 2016
@vutny
Copy link
Contributor Author

vutny commented Mar 9, 2016

Ping @s0undt3ch

s0undt3ch added a commit that referenced this pull request Mar 9, 2016
Fix multiple lint errors (shellcheck) and make some refactoring
@s0undt3ch s0undt3ch merged commit 6f5ea12 into saltstack:develop Mar 9, 2016
@s0undt3ch
Copy link
Member

Sorry about the slowness.

@vutny vutny deleted the fix-lint-errors branch March 10, 2016 09:07
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 this pull request may close these issues.

None yet

2 participants