You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/04/27 15:03:15 UTC

[GitHub] [beam] jrmccluskey opened a new pull request, #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

jrmccluskey opened a new pull request, #17479:
URL: https://github.com/apache/beam/pull/17479

   Adds Staticcheck to Github Actions precommits, adding another check for common issues in Go code. Disables three checks that are usually used by default: SA1019 (deprecation warnings,) U1000 (unused fields, vars, and functions,) and S1021 (merge variable declaration and assignment) with the latter two intended to be re-enabled once their instances are resolved.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] jrmccluskey commented on a diff in pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on code in PR #17479:
URL: https://github.com/apache/beam/pull/17479#discussion_r859949592


##########
.github/workflows/go_tests.yml:
##########
@@ -53,3 +53,7 @@ jobs:
         run: cd sdks/go/pkg/beam && go fmt ./...; git diff-index --quiet HEAD || (echo "Run go fmt before checking in changes" && exit 1)
       - name: Run vet
         run: cd sdks/go/pkg/beam && go vet --copylocks=false --unsafeptr=false ./... || (echo "Run go vet and fix warnings before checking in changes" && exit 1)
+      - uses: dominikh/staticcheck-action@v1.1.0
+        with:
+	  version: "2022.1.0"
+          install-go: false

Review Comment:
   That tracks, I quickly threw the change together in Vim and getting the whitespace aligned was giving me fits



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] lostluck commented on pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
lostluck commented on PR #17479:
URL: https://github.com/apache/beam/pull/17479#issuecomment-1113517389

   Looks like the action isn't whitelisted by INFRA yet. I've filed a JIRA ticket https://issues.apache.org/jira/browse/INFRA-23219 with them, and merge once it's working.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] jrmccluskey commented on a diff in pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on code in PR #17479:
URL: https://github.com/apache/beam/pull/17479#discussion_r861967187


##########
sdks/go/pkg/beam/staticcheck.conf:
##########
@@ -0,0 +1,21 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# TODO(BEAM-14371): Clean up instances of S1021 (merge variable declaration and assignment) and enable check
+# TODO(BEAM-14372): Clean up instances of U1000 (unused fields, vars, functions) and enable check
+checks = ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-ST1023", "-SA1019", "-U1000", "-S1021"]
+initialisms = ["ACL", "API", "ASCII", "CPU", "CSS", "DNS", "EOF", "GUID", "HTML", "HTTP", "HTTPS", "ID", "IP", "JSON", "QPS", "RAM", "RPC", "SLA", "SMTP", "SQL", "SSH", "TCP", "TLS", "TTL", "UDP", "UI", "GID", "UID", "UUID", "URI", "URL", "UTF8", "VM", "XML", "XMPP", "XSRF", "XSS", "SIP", "RTP", "AMQP", "DB", "TS"]
+dot_import_whitelist = ["github.com/mmcloughlin/avo/build", "github.com/mmcloughlin/avo/operand", "github.com/mmcloughlin/avo/reg"]

Review Comment:
   Fair enough, removing those extra fields doesn't impact our results from the tool anyway



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] jrmccluskey commented on pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on PR #17479:
URL: https://github.com/apache/beam/pull/17479#issuecomment-1113509486

   The Apache security config for actions doesn't like the third party action. Looks like I will need to write it manually instead, do not merge until that's updated 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] jrmccluskey commented on pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on PR #17479:
URL: https://github.com/apache/beam/pull/17479#issuecomment-1111129740

   R: @damccorm @lostluck 
   
   The workflow will need to be run manually on the PR to make sure this works correctly but I don't have permissions to do so.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] damccorm commented on a diff in pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #17479:
URL: https://github.com/apache/beam/pull/17479#discussion_r859938715


##########
.github/workflows/go_tests.yml:
##########
@@ -53,3 +53,7 @@ jobs:
         run: cd sdks/go/pkg/beam && go fmt ./...; git diff-index --quiet HEAD || (echo "Run go fmt before checking in changes" && exit 1)
       - name: Run vet
         run: cd sdks/go/pkg/beam && go vet --copylocks=false --unsafeptr=false ./... || (echo "Run go vet and fix warnings before checking in changes" && exit 1)
+      - uses: dominikh/staticcheck-action@v1.1.0
+        with:
+	  version: "2022.1.0"
+          install-go: false

Review Comment:
   ```suggestion
             version: "2022.1.0"
             install-go: false
   ```
   
   The diff showed some weird spacing here (I think non-space/tab characters?), that might keep actions from being able to parse this file (and is just better to not have)



##########
.github/workflows/go_tests.yml:
##########
@@ -53,3 +53,7 @@ jobs:
         run: cd sdks/go/pkg/beam && go fmt ./...; git diff-index --quiet HEAD || (echo "Run go fmt before checking in changes" && exit 1)
       - name: Run vet
         run: cd sdks/go/pkg/beam && go vet --copylocks=false --unsafeptr=false ./... || (echo "Run go vet and fix warnings before checking in changes" && exit 1)
+      - uses: dominikh/staticcheck-action@v1.1.0

Review Comment:
   What is the advantage of using a 3rd party action over a script that calls static check directly? 
   
   The reason I ask is that in general, I think we should prefer the latter if there's not a reason we need an action. That makes it easier for users to tell exactly what is being run/incorporate it into their own workflow, plus it reduces our surface area for bugs/security issues. If there's a good reason to use the action I'm 100% on board though



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] codecov[bot] commented on pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #17479:
URL: https://github.com/apache/beam/pull/17479#issuecomment-1113639912

   # [Codecov](https://codecov.io/gh/apache/beam/pull/17479?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17479](https://codecov.io/gh/apache/beam/pull/17479?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (81d7fd8) into [master](https://codecov.io/gh/apache/beam/commit/64dc9c62dce2e5af1f52c93a04702f17bfa89a60?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (64dc9c6) will **decrease** coverage by `0.03%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17479      +/-   ##
   ==========================================
   - Coverage   73.88%   73.85%   -0.04%     
   ==========================================
     Files         690      690              
     Lines       90761    90829      +68     
   ==========================================
   + Hits        67062    67084      +22     
   - Misses      22490    22532      +42     
   - Partials     1209     1213       +4     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | go | `50.01% <ø> (-0.05%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/17479?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/go/pkg/beam/core/runtime/exec/datasource.go](https://codecov.io/gh/apache/beam/pull/17479/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9kYXRhc291cmNlLmdv) | `65.47% <0.00%> (-6.22%)` | :arrow_down: |
   | [sdks/go/pkg/beam/core/runtime/exec/plan.go](https://codecov.io/gh/apache/beam/pull/17479/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9wbGFuLmdv) | `46.97% <0.00%> (-1.64%)` | :arrow_down: |
   | [sdks/go/pkg/beam/core/runtime/harness/harness.go](https://codecov.io/gh/apache/beam/pull/17479/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | `10.46% <0.00%> (-0.49%)` | :arrow_down: |
   | [sdks/go/pkg/beam/core/runtime/exec/fn.go](https://codecov.io/gh/apache/beam/pull/17479/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9mbi5nbw==) | `62.41% <0.00%> (+1.63%)` | :arrow_up: |
   | [sdks/go/pkg/beam/core/runtime/exec/coder.go](https://codecov.io/gh/apache/beam/pull/17479/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9jb2Rlci5nbw==) | `58.22% <0.00%> (+1.75%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17479?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/17479?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [64dc9c6...81d7fd8](https://codecov.io/gh/apache/beam/pull/17479?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] lostluck commented on a diff in pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
lostluck commented on code in PR #17479:
URL: https://github.com/apache/beam/pull/17479#discussion_r861962038


##########
sdks/go/pkg/beam/staticcheck.conf:
##########
@@ -0,0 +1,21 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# TODO(BEAM-14371): Clean up instances of S1021 (merge variable declaration and assignment) and enable check
+# TODO(BEAM-14372): Clean up instances of U1000 (unused fields, vars, functions) and enable check
+checks = ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-ST1023", "-SA1019", "-U1000", "-S1021"]
+initialisms = ["ACL", "API", "ASCII", "CPU", "CSS", "DNS", "EOF", "GUID", "HTML", "HTTP", "HTTPS", "ID", "IP", "JSON", "QPS", "RAM", "RPC", "SLA", "SMTP", "SQL", "SSH", "TCP", "TLS", "TTL", "UDP", "UI", "GID", "UID", "UUID", "URI", "URL", "UTF8", "VM", "XML", "XMPP", "XSRF", "XSS", "SIP", "RTP", "AMQP", "DB", "TS"]
+dot_import_whitelist = ["github.com/mmcloughlin/avo/build", "github.com/mmcloughlin/avo/operand", "github.com/mmcloughlin/avo/reg"]

Review Comment:
   Default configuration that `staticcheck` uses doesn't mean it's appropriate for our project though.
   
   Technically, I think we should simply remove the
   `initialisms`, `dot_import_whitelist`, and `http_status_code_whitelist` since we aren't using anything more than the defaults anyway, and it removes the implication we're relying on these specific configurations.
   We certainly should keep the `checks` as we've previously discussed.
   
   We don't (and shouldn't) have dot imports in the SDK, and we don't use these packages ourselves, so we shouldn't allow this gap to have them introduced.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] asf-ci commented on pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17479:
URL: https://github.com/apache/beam/pull/17479#issuecomment-1111113464

   Can one of the admins verify this patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] github-actions[bot] commented on pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #17479:
URL: https://github.com/apache/beam/pull/17479#issuecomment-1111186698

   Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
   
   R: @lostluck for label go.
   
   Available commands:
   - `stop reviewer notifications` - opt out of the automated review tooling
   - `remind me after tests pass` - tag the comment author after tests pass
   - `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
   
   The PR bot will only process comments in the main thread (not review comments).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] lostluck commented on a diff in pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
lostluck commented on code in PR #17479:
URL: https://github.com/apache/beam/pull/17479#discussion_r861980949


##########
.github/workflows/go_tests.yml:
##########
@@ -53,3 +53,7 @@ jobs:
         run: cd sdks/go/pkg/beam && go fmt ./...; git diff-index --quiet HEAD || (echo "Run go fmt before checking in changes" && exit 1)
       - name: Run vet
         run: cd sdks/go/pkg/beam && go vet --copylocks=false --unsafeptr=false ./... || (echo "Run go vet and fix warnings before checking in changes" && exit 1)
+      - uses: dominikh/staticcheck-action@v1.1.0

Review Comment:
   Nvm. We should probably punt that until after we move to 1.18 properly.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] lostluck merged pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
lostluck merged PR #17479:
URL: https://github.com/apache/beam/pull/17479


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] jrmccluskey commented on a diff in pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on code in PR #17479:
URL: https://github.com/apache/beam/pull/17479#discussion_r861367834


##########
sdks/go/pkg/beam/staticcheck.conf:
##########
@@ -0,0 +1,21 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# TODO(BEAM-14371): Clean up instances of S1021 (merge variable declaration and assignment) and enable check
+# TODO(BEAM-14372): Clean up instances of U1000 (unused fields, vars, functions) and enable check
+checks = ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-ST1023", "-SA1019", "-U1000", "-S1021"]
+initialisms = ["ACL", "API", "ASCII", "CPU", "CSS", "DNS", "EOF", "GUID", "HTML", "HTTP", "HTTPS", "ID", "IP", "JSON", "QPS", "RAM", "RPC", "SLA", "SMTP", "SQL", "SSH", "TCP", "TLS", "TTL", "UDP", "UI", "GID", "UID", "UUID", "URI", "URL", "UTF8", "VM", "XML", "XMPP", "XSRF", "XSS", "SIP", "RTP", "AMQP", "DB", "TS"]
+dot_import_whitelist = ["github.com/mmcloughlin/avo/build", "github.com/mmcloughlin/avo/operand", "github.com/mmcloughlin/avo/reg"]

Review Comment:
   This is the actual configuration file that staticcheck uses. This is the default config (https://staticcheck.io/docs/configuration/#example-configuration) with the addition of skipping three other checks. In the event that we cleaned up the extra warnings from things like deprecations we could remove this, but as we've discussed the deprecation warnings are a decently sized project and getting some signal from this sooner is better



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] asf-ci commented on pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17479:
URL: https://github.com/apache/beam/pull/17479#issuecomment-1111113455

   Can one of the admins verify this patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] asf-ci commented on pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17479:
URL: https://github.com/apache/beam/pull/17479#issuecomment-1111113456

   Can one of the admins verify this patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] jrmccluskey commented on a diff in pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on code in PR #17479:
URL: https://github.com/apache/beam/pull/17479#discussion_r859952348


##########
.github/workflows/go_tests.yml:
##########
@@ -53,3 +53,7 @@ jobs:
         run: cd sdks/go/pkg/beam && go fmt ./...; git diff-index --quiet HEAD || (echo "Run go fmt before checking in changes" && exit 1)
       - name: Run vet
         run: cd sdks/go/pkg/beam && go vet --copylocks=false --unsafeptr=false ./... || (echo "Run go vet and fix warnings before checking in changes" && exit 1)
+      - uses: dominikh/staticcheck-action@v1.1.0

Review Comment:
   The action is direct from the maintainers of staiccheck and is the recommended way to include it in actions (https://staticcheck.io/docs/running-staticcheck/ci/github-actions/). Staticcheck isn't built-in to Go unlike fmt and vet so offloading the work of downloading and configuring the tool to the third-party action seems reasonable



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] damccorm commented on a diff in pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #17479:
URL: https://github.com/apache/beam/pull/17479#discussion_r859962691


##########
.github/workflows/go_tests.yml:
##########
@@ -53,3 +53,7 @@ jobs:
         run: cd sdks/go/pkg/beam && go fmt ./...; git diff-index --quiet HEAD || (echo "Run go fmt before checking in changes" && exit 1)
       - name: Run vet
         run: cd sdks/go/pkg/beam && go vet --copylocks=false --unsafeptr=false ./... || (echo "Run go vet and fix warnings before checking in changes" && exit 1)
+      - uses: dominikh/staticcheck-action@v1.1.0

Review Comment:
   Cool, that was the context I was looking for - thanks! SGTM



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] lostluck commented on a diff in pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
lostluck commented on code in PR #17479:
URL: https://github.com/apache/beam/pull/17479#discussion_r861366615


##########
sdks/go/pkg/beam/staticcheck.conf:
##########
@@ -0,0 +1,21 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# TODO(BEAM-14371): Clean up instances of S1021 (merge variable declaration and assignment) and enable check
+# TODO(BEAM-14372): Clean up instances of U1000 (unused fields, vars, functions) and enable check
+checks = ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-ST1023", "-SA1019", "-U1000", "-S1021"]
+initialisms = ["ACL", "API", "ASCII", "CPU", "CSS", "DNS", "EOF", "GUID", "HTML", "HTTP", "HTTPS", "ID", "IP", "JSON", "QPS", "RAM", "RPC", "SLA", "SMTP", "SQL", "SSH", "TCP", "TLS", "TTL", "UDP", "UI", "GID", "UID", "UUID", "URI", "URL", "UTF8", "VM", "XML", "XMPP", "XSRF", "XSS", "SIP", "RTP", "AMQP", "DB", "TS"]
+dot_import_whitelist = ["github.com/mmcloughlin/avo/build", "github.com/mmcloughlin/avo/operand", "github.com/mmcloughlin/avo/reg"]

Review Comment:
   what does this configuration do?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] lostluck commented on a diff in pull request #17479: [BEAM-14243] Add staticcheck to Github Actions Precommits

Posted by GitBox <gi...@apache.org>.
lostluck commented on code in PR #17479:
URL: https://github.com/apache/beam/pull/17479#discussion_r861974776


##########
.github/workflows/go_tests.yml:
##########
@@ -53,3 +53,7 @@ jobs:
         run: cd sdks/go/pkg/beam && go fmt ./...; git diff-index --quiet HEAD || (echo "Run go fmt before checking in changes" && exit 1)
       - name: Run vet
         run: cd sdks/go/pkg/beam && go vet --copylocks=false --unsafeptr=false ./... || (echo "Run go vet and fix warnings before checking in changes" && exit 1)
+      - uses: dominikh/staticcheck-action@v1.1.0

Review Comment:
   The staticcheck docs say 1.1, but it looks like 1.2 is the latest version. We may as well be on the current version.
   
   https://github.com/marketplace/actions/staticcheck



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org