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

bin/runlinter.sh broken #471

Closed
1 of 4 tasks
ldemailly opened this issue Jul 16, 2017 · 7 comments
Closed
1 of 4 tasks

bin/runlinter.sh broken #471

ldemailly opened this issue Jul 16, 2017 · 7 comments

Comments

@ldemailly
Copy link
Contributor

ldemailly commented Jul 16, 2017

used to work before #463

  • bin/linter.sh -h yields bin/linters.sh: line 19: error_exit: command not found (all OSes)
  • bin/linters.sh: line 62: declare: -A: invalid option when using -s HEAD on mac
  • it also doesn't seem to work even on linux / select touched files (see log below)
  • it should get the go packages list from bazel

Details : problem 2:

Running linters
All known packages are ./tests/e2e/... ./devel/githubContrib
Using HEAD to compare files to.
bin/linters.sh: line 62: declare: -A: invalid option
declare: usage: declare [-afFirtx] [-p] [name[=value] ...]

Mac/old bash only problem I guess

Details: Problem 3:

$ bin/linter.sh -s 3a429c7dae6fe1361fbfe525b4ba9eab311538ff
Running linters
All known packages are ./tests/e2e/... ./devel/githubContrib
Using 3a429c7dae6fe1361fbfe525b4ba9eab311538ff to compare files to.
Running linters on packages .
Done running linters
ldemailly@benchmark-1:~/go/src/istio.io/istio$ git diff 3a429c7dae6fe1361fbfe525b4ba9eab311538ff
diff --git a/devel/fortio/stats.go b/devel/fortio/stats.go
index 44f6e92..10a1139 100644
--- a/devel/fortio/stats.go
+++ b/devel/fortio/stats.go
@@ -292,7 +292,7 @@ func (h *Histogram) Log(msg string, percentile float64) {
        var b bytes.Buffer
        w := bufio.NewWriter(&b)
        h.Print(w, msg, percentile)
-       w.Flush() // nolint: gas,errcheck
+       w.Flush()
        log.Print(string(b.Bytes()))
 }
 
@sebastienvas
Copy link
Contributor

sebastienvas commented Jul 16, 2017 via email

@ldemailly
Copy link
Contributor Author

that explains one problem but where is that done ? why a bazel build not enough?

@ldemailly
Copy link
Contributor Author

ldemailly commented Jul 17, 2017

oh I read the script, there are 2 packages there - I thought there was some explicit list somewhere and it was printing a summary

any way to get the actual package list from bazel instead of doing /... which is very coarse?

ldemailly added a commit that referenced this issue Jul 17, 2017
The granularity seems not great no ?
ldemailly added a commit that referenced this issue Jul 22, 2017
ldemailly added a commit that referenced this issue Jul 24, 2017
…setup_run (#495)

* Document how to add dependencies + linter fix + small improvement to setup_run

Partial fix for #471

* Updating the doc about linter.sh run
@ldemailly
Copy link
Contributor Author

ping...

@sebastienvas
Copy link
Contributor

sebastienvas commented Aug 4, 2017

Feel free to give it a go. This is is definitely not on priority list.

@ldemailly ldemailly added this to the Istio 0.2 milestone Aug 16, 2017
@linsun linsun modified the milestones: Istio 0.2, Istio 0.3 Sep 28, 2017
@linsun
Copy link
Member

linsun commented Sep 28, 2017

not a stop ship for v0.2

mandarjog pushed a commit to mandarjog/istio that referenced this issue Oct 30, 2017
- Eliminate a useless abstraction layer. Now that there's a clean
AdapterDispatcher interface, we don't really need the extra Handlers
concept in pkg/api.

Former-commit-id: 72400294dc7bd5752e8ede9117473ab97847d626
rshriram pushed a commit that referenced this issue Oct 30, 2017
…setup_run (#495)

* Document how to add dependencies + linter fix + small improvement to setup_run

Partial fix for #471

* Updating the doc about linter.sh run


Former-commit-id: 35d8505
mandarjog pushed a commit that referenced this issue Oct 31, 2017
- Eliminate a useless abstraction layer. Now that there's a clean
AdapterDispatcher interface, we don't really need the extra Handlers
concept in pkg/api.

Former-commit-id: 12e3448ead8fae61a45fb6211b79ffa715a8f1c9
vbatts pushed a commit to vbatts/istio that referenced this issue Oct 31, 2017
…setup_run (istio#495)

* Document how to add dependencies + linter fix + small improvement to setup_run

Partial fix for istio#471

* Updating the doc about linter.sh run


Former-commit-id: 35d8505
mandarjog pushed a commit that referenced this issue Nov 2, 2017
…setup_run (#495)

* Document how to add dependencies + linter fix + small improvement to setup_run

Partial fix for #471

* Updating the doc about linter.sh run


Former-commit-id: 35d8505
@sebastienvas sebastienvas removed their assignment Nov 3, 2017
@ldemailly
Copy link
Contributor Author

it works now

howardjohn pushed a commit to howardjohn/istio that referenced this issue Jan 12, 2020
* Turn off DNS certificate provisioning

* Run "make gen" and include the changes

* make gen after unset hub and tag
luksa pushed a commit to luksa/istio that referenced this issue Sep 20, 2022
Co-authored-by: John Howard <howardjohn@google.com>
antonioberben pushed a commit to antonioberben/istio that referenced this issue Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants