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.

Sign up for GitHub

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

Jump to bottom

.coafile: Add ShellCheckBear #2385

Merged

gitmate-bot merged 2 commits into coala:master from sangamcse:shellcheckbear

Apr 10, 2018

Member

sangamcse commented Mar 28, 2018 •

edited

Loading

This add a bash section and bash
as default shell in .coafile.

Closes #2320

For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!

Checklist

I read the commit guidelines and I've followed
them.
I ran coala over my code locally. (All commits have to pass
individually. It is not sufficient to have "fixup commits" on your PR,
our bot will still report the issues for the previous commit.) You will
likely receive a lot of bot comments and build failures if coala does not
pass on every single commit!

After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.

Please consider helping us by reviewing other peoples pull requests as well:

pick up any PR at https://coala.io/review
review it (check https://coala.io/reviewing for more info)
if you are sure that it needs work, use corobo mark wip <URL> to get it out
of the review queue.

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

gitmate-bot added process/pending review size/S difficulty/low labels

gitmate-bot assigned sangamcse

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 77d9829, file .ci/env_variables.sh, line 1.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 77d9829, file .ci/env_variables.sh, line 2.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 77d9829, file .ci/env_variables.sh, line 3.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

sangamcse force-pushed the shellcheckbear branch from fe33730 to 870f058 Compare

March 28, 2018 13:17

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 64e11fb, file .ci/env_variables.sh, line 1.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 64e11fb, file .ci/env_variables.sh, line 2.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 64e11fb, file .ci/env_variables.sh, line 3.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 77d9829, file .ci/env_variables.sh, line 1.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 77d9829, file .ci/env_variables.sh, line 2.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 77d9829, file .ci/env_variables.sh, line 3.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

sangamcse force-pushed the shellcheckbear branch from 870f058 to 03c39a2 Compare

March 28, 2018 13:25

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 77d9829, file .ci/env_variables.sh, line 1.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 77d9829, file .ci/env_variables.sh, line 2.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

Collaborator

gitmate-bot commented Mar 28, 2018

Comment on 77d9829, file .ci/env_variables.sh, line 3.

Declare and assign separately to avoid masking return values. [SC2155]

Origin: ShellCheckBear, Section: bash.

sangamcse force-pushed the shellcheckbear branch 2 times, most recently from 86c10d7 to 105d991 Compare

March 28, 2018 14:43

gitmate-bot added size/XS and removed size/S labels

refeed reviewed

View reviewed changes

Member

refeed left a comment

Mostly LGTM 👍 , one thing again, could you squash your commits into one commit (.coafile: Add ShellCheckBear), since they're still in the same topic

Member Author

sangamcse commented Mar 30, 2018

@refeed, Thank you for your review. As you can see the commit body's comment, I have mentioned that the file inside .ci was modified as per shfmt and not using shellcheck. So I have splitted these commit.
Also, check coala/coala#5243 commits

Member

refeed commented Mar 30, 2018

Ahh, makes sense, I thought shfmt was a tool that is related with shellcheck.

refeed reviewed

View reviewed changes

.ci/package.json.jinja2 Outdated

@@ @@ -3,4 +3,3 @@ @@
                 "version": "{{version}}",
                 "dependencies": {{dependencies | jsonify(indent=4, sort_keys=True)}}
               }

Member

refeed Mar 30, 2018

I think this is not related with #2320 ?

Member Author

sangamcse Mar 30, 2018

Yes. Should I make it newcomer issue or keep it here? I found this while I was modifying shell scripts in .ci.

Member

refeed Mar 30, 2018

yup, I think a newcomer issue would be fine for this

Member Author

sangamcse Mar 30, 2018

Done #2388

sangamcse force-pushed the shellcheckbear branch 4 times, most recently from d54a65b to 05c375e Compare

March 30, 2018 10:09

sangamcse force-pushed the shellcheckbear branch from 05c375e to 48f98fc Compare

March 30, 2018 10:15

refeed reviewed

View reviewed changes

Member

refeed left a comment

Btw, where is the line that does:

Also, add `shellcheck` as prerequisite
for CircleCI.

Member Author

sangamcse commented Mar 30, 2018

@refeed, good catch 😉. I just removed that line before your comment.

refeed approved these changes

View reviewed changes

Member

refeed left a comment

LGTM

jayvdb reviewed

View reviewed changes

.ci/deps.r.sh Outdated

@@ @@ -2,7 +2,7 @@ set -e @@
               set -x
               # R commands
-              echo '.libPaths( c( "'"$R_LIB_USER"'", .libPaths()) )' >> .Rprofile
-              echo 'options(repos=structure(c(CRAN="http://cran.rstudio.com")))' >> .Rprofile
+              echo '.libPaths( c( "'"$R_LIB_USER"'", .libPaths()) )' >>.Rprofile

Member

jayvdb Apr 7, 2018

hmm. I find this style very jarring.
can it be disabled?
@yukiisbored your opinion on this rule?

Member Author

sangamcse Apr 9, 2018

I don't have much knowledge about these R stuff.

Member

jayvdb Apr 10, 2018

It isnt R.

All >> foo are being forced to be >>foo

Member

jayvdb Apr 10, 2018

Can you find a rule to ignore these changes?

Member Author

sangamcse Apr 10, 2018 •

edited

Loading

Understood.
Actually, this was not done by shellcheck but by the shfmt.

can it be disabled?

I can revert these changes. 😃

jayvdb reviewed

View reviewed changes

.ci/deps.pip.sh Outdated

@@ @@ -5,13 +5,13 @@ TERM=dumb @@
               # Choose the python versions to install deps for
               case $CIRCLE_NODE_INDEX in
-) dep_versions=( "3.4.3" "3.5.1" ) ;;

Member

jayvdb Apr 7, 2018

all this circle stuff will go with the circle-ci patch.
but that might take a while.

Member Author

sangamcse Apr 9, 2018

Should I keep this file or remove all changes?

Member

jayvdb Apr 10, 2018

Put it on the ignore list.

.ci/check_maintainership.sh Outdated

		@@ -8,6 +8,6 @@ source ../rultor_secrets.sh

Member

jayvdb Apr 7, 2018

this script will be removed as part of see #2410
should happen quickly

Member Author

sangamcse Apr 8, 2018 •

edited

Loading

Should I remove any changes from this file? @jayvdb

Member

jayvdb Apr 8, 2018

That is fixed; rebase and this file disappears

sangamcse force-pushed the shellcheckbear branch 5 times, most recently from d0c74f8 to 0b67368 Compare

April 10, 2018 09:06

jayvdb requested changes

View reviewed changes

.ci/deps.apt.sh Outdated

@@ @@ -19,7 +19,7 @@ case $CIRCLE_BUILD_IMAGE in @@
                   # Use xenial, needed to replace outdated julia provided by Circle CI
                   ADD_APT_UBUNTU_RELEASE=xenial
                   # Work around lack of systemd on trusty, which xenial's lxc-common expects
-                  echo '#!/bin/sh' | sudo tee /usr/bin/systemd-detect-virt > /dev/null
+                  echo '#!/bin/sh' | sudo tee /usr/bin/systemd-detect-virt >/dev/null

Member

jayvdb Apr 10, 2018

cant we ignore the rule that causes this?

Member Author

sangamcse Apr 10, 2018

Yes.

sangamcse added 2 commits

April 10, 2018 16:00


          .coafile: Add ShellCheckBear

This add a `bash` section and `bash`
as default `shell` in `.coafile`.

Closes coala#2320


          .ci: Modify shell scripts to two space indents

b650a4b

Give coala's shell scripts a standard.
Modify .sh files using `shfmt -i 2 -ci`
from https://github.com/mvdan/sh#shfmt.
This changes shell scripts according to
Google's guide.

Closes coala#2320

sangamcse force-pushed the shellcheckbear branch from 0b67368 to b650a4b Compare

April 10, 2018 10:33

jayvdb approved these changes

View reviewed changes

Member

jayvdb commented Apr 10, 2018

ack 7634540 b650a4b

Member

jayvdb commented Apr 10, 2018

@gitmate-bot ff

Collaborator

gitmate-bot commented Apr 10, 2018

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

Collaborator

gitmate-bot commented Apr 10, 2018

Automated fastforward with GitMate.io was successful! 🎉

gitmate-bot merged commit b650a4b into coala:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

difficulty/low process/pending review size/XS