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 linter errors #2068

Merged
merged 7 commits into from
Jan 18, 2017
Merged

Conversation

iaguis
Copy link
Contributor

@iaguis iaguis commented Dec 7, 2016

@iaguis iaguis changed the title Fix linter error Fix linter errors Dec 7, 2016
@2opremio
Copy link
Contributor

2opremio commented Dec 12, 2016

Please don't modify tools directory in-place (it's a git tree).

Also, I believe that generate_latest_map if anything, should live on /extras (we may not really need it though).

Would you mind creating a PR to remove it in https://github.com/weaveworks/build-tools and another one to bump the git-tree (and optionally add the script to extras) in the repository?

Thanks

@2opremio
Copy link
Contributor

@iaguis ping

iaguis added 2 commits January 9, 2017 14:40
4b7d5c6 Merge pull request weaveworks#59 from weaveworks/57-fix-lint-properly
b7f0e69 Merge pull request weaveworks#58 from weaveworks/fix-lint
794702c Pin version of shfmt
ab1b11d Fix lint
81d80f3 Merge pull request weaveworks#55 from weaveworks/lint-tf
05ad5f2 Review feedback
4c0d046 Use hclfmt to lint terraform.
fd875e2 Fix test wrt shellcheck
54ec2d9 Don't capitalise error messages
19d3b6e Merge pull request weaveworks#49 from weaveworks/pin-shfmt
fea98f6 Go get from the vendor dir
1d867b0 Try and vendor a specific version of shfmt
76619c2 Merge pull request weaveworks#48 from weaveworks/revert-41-user-tokens
4f96c51 Revert "Add experimental support for user tokens"
d00033f Merge pull request weaveworks#41 from weaveworks/user-tokens
245ed26 Merge pull request weaveworks#47 from weaveworks/46-shfmt
c1d7815 Fix shfmt error
cb39746 Don't overright lint_result with 0 when shellcheck succeeds
8ab80e8 Merge pull request weaveworks#45 from weaveworks/lint
83d5bd1 getting integration/config and test shellcheck-compliant
cff9ec3 Fix some shellcheck errors
7a843d6 run shellcheck as part of lint if it is installed
31552a0 removing spurious space from test
6ca7c5f Merge pull request weaveworks#44 from weaveworks/shfmt
952356d Allow lint to lint itself
b7ac59c Run shfmt on all shell files in this repo
5570b0e Add shfmt formatting of shell files in lint
0a67594 fix circle build by splatting gopath permissions
354e083 Fixing lint
586060b Add experimental support for user tokens

git-subtree-dir: tools
git-subtree-split: 4b7d5c6
@iaguis
Copy link
Contributor Author

iaguis commented Jan 9, 2017

Sorry for letting this slip through the cracks.

It seems generate_latest_map is already removed from build-tools. I updated this PR with a build-tools bump and re-added the script to extras/.

@iaguis
Copy link
Contributor Author

iaguis commented Jan 9, 2017

Hm, and now I get tons of shellcheck errors

iaguis added 3 commits January 9, 2017 16:40
It seems it was added directly to the scope repo instead of adding it to
the build-tools repo so we need to delete it manually.
It was removed from the build-tools repository. Add it to extra/ and fix
a couple of linter errors.
@iaguis iaguis force-pushed the iaguis/fix-lint branch 3 times, most recently from 4201e71 to 3bf7747 Compare January 10, 2017 16:55
@iaguis
Copy link
Contributor Author

iaguis commented Jan 10, 2017

Not sure what the problem is now. The error is not very useful:

$ cd $SRCDIR; make RM= lint
scope: run shfmt -i 4 -w scope

Makefile:105: recipe for target 'lint' failed

make: *** [lint] Error 1

make: Leaving directory '/go/src/github.com/weaveworks/scope'

make: *** [lint] Error 2

cd $SRCDIR; make RM= lint returned exit code 2

@iaguis iaguis force-pushed the iaguis/fix-lint branch 4 times, most recently from 45dcff9 to 7d75d8e Compare January 11, 2017 17:56
@iaguis
Copy link
Contributor Author

iaguis commented Jan 12, 2017

The problems were shfmt issues. PTAL.

@@ -26,10 +23,10 @@ infer_release_type() {
setup() {
# Ensure we have exactly one annotated tag pointing at HEAD
HEAD_TAGS=$(git tag --points-at HEAD)
TAG_COUNT=$(echo $(echo $HEAD_TAGS | wc -w)) # mac hack
TAG_COUNT=$(echo "$HEAD_TAGS" | wc -w) # mac hack

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@2opremio
Copy link
Contributor

Thanks a lot for this @iaguis , it must have been a pain ...

@2opremio 2opremio merged commit 43adb03 into weaveworks:master Jan 18, 2017
@2opremio
Copy link
Contributor

Crap, I missed there are linter fixes in tools/lint, which should had been done in build-tools

@iaguis
Copy link
Contributor Author

iaguis commented Jan 18, 2017

Oh noes, I missed that too 😞

@2opremio
Copy link
Contributor

Again, please remember that tools is a subtree

@2opremio
Copy link
Contributor

I will use the file version from build-tools, if the linter keeps bitching we can fix it there.

alban added a commit to kinvolk-archives/scope that referenced this pull request Jan 30, 2017
Symptoms: the Scope URL are printed this way:

```
Weave Scope is reachable at the following URL(s):
  * http://10.2.2.1
172.16.28.1
172.16.28.1
192.168.99.1
192.168.35.117
172.16.28.1
172.16.28.1
192.168.122.1:4040/
```

Regression from weaveworks#2068
@alban alban mentioned this pull request Jan 30, 2017
@@ -2,4 +2,4 @@

set -e

./in_parallel.sh "make RM=" $(find . -maxdepth 2 -name *.go -printf "%h\n" | sort -u | sed -n 's/\.\/\(.*\)/\1\/\1/p')
./in_parallel.sh "make RM=" "$(find . -maxdepth 2 -name "./*.go" -printf "%h\n" | sort -u | sed -n 's/\.\/\(.*\)/\1\/\1/p')"

This comment was marked as abuse.

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.

3 participants