From 76088ef0cd4e666961c1f551bc4a853b15434445 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 2 Sep 2021 00:11:42 -0700 Subject: [PATCH 1/9] Place project-specific settings first in codespell configuration The `ignore-words-list` and `skip` settings of the codespell configuration file may required project-specific adjustments to fix false positives or avoid positives from externally maintained files. The other settings are universal. It will be convenient to have the settings the user might need to adjust at the place of highest visibility in the configuration file. --- .codespellrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.codespellrc b/.codespellrc index 83ec5bfb..f5cbc5a3 100644 --- a/.codespellrc +++ b/.codespellrc @@ -3,7 +3,7 @@ [codespell] # In the event of a false positive, add the problematic word, in all lowercase, to a comma-separated list here: ignore-words-list = ot,propert +skip = ./.git,**/go.mod,**/go.sum,./package-lock.json,./poetry.lock,./yarn.lock,./arduino-lint,./arduino-lint.exe,./internal/rule/rulefunction/testdata/libraries/MisspelledSentenceParagraphValue/library.properties,./site builtin = clear,informal,en-GB_to_en-US check-filenames = check-hidden = -skip = ./.git,**/go.mod,**/go.sum,./package-lock.json,./poetry.lock,./yarn.lock,./arduino-lint,./arduino-lint.exe,./internal/rule/rulefunction/testdata/libraries/MisspelledSentenceParagraphValue/library.properties,./site From befd449107ebf631ab0c439e04d0119ef1b6eec7 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 2 Sep 2021 00:12:05 -0700 Subject: [PATCH 2/9] Make editorconfig-checker exclude regexes explicit The `Exclude` array in the `.ecrc` configuration file contains the regular expressions of paths that should be excluded from checking by the editorconfig-checker tool. Previously, the "template" file contained a list of filenames that should always be ignored. Although it did work, it was not really correct because the `.` in the filename is actually a regex wildcard, which is not what was intended. Although a false match was unlikely, this also might mislead users adding project-specific exclusions to the file regarding the nature of the exclude patterns. Changing to very explicit patterns avoids any chance of false matches and also makes it clear that these are regexes. --- .ecrc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.ecrc b/.ecrc index b6366684..c1a94d0c 100644 --- a/.ecrc +++ b/.ecrc @@ -1,7 +1,7 @@ { "Exclude": [ - "LICENSE.txt", - "poetry.lock", + "^LICENSE\\.txt$", + "^poetry\\.lock$", "^internal/rule/schema/schemadata/bindata.go$", "^internal/rule/schema/testdata/bindata.go$" ] From 80ee25da588f765dacef7ed89b0402bb5f9a7ee2 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 2 Sep 2021 00:12:34 -0700 Subject: [PATCH 3/9] Add `.gitconfig` file type to `.editorconfig` The `.gitmodules` file defines the properties of a repository's submodules. The file automatically generated by Git submodule commands use tabs for indentation. It uses the same file format as the Git configuration file (e.g., `.gitconfig`). Even though the `.gitconfig` file is not likely to be found under the repository tree, it's possible the `.editorconfig` might end up being used outside the project specific scope so I added it to the file pattern. --- .editorconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.editorconfig b/.editorconfig index 99945e92..eda85443 100644 --- a/.editorconfig +++ b/.editorconfig @@ -56,5 +56,5 @@ indent_style = space indent_size = 2 indent_style = space -[.gitmodules] +[{.gitconfig,.gitmodules}] indent_style = tab From 7d225e46c071aa3e98a8f537d9db72a552164e94 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 2 Sep 2021 00:15:07 -0700 Subject: [PATCH 4/9] Improve readability of macOS repackaging commands Some complex and lengthy commands are used to repackage the macOS build of the Go application after notarization. Adding some escaped line breaks and indenting the line continuations to indicate the command structure makes it easier to read and understand. This was already done for the other complex commands of the workflows, but was neglected for the repackaging step. --- .github/workflows/publish-go-nightly-task.yml | 10 +++++++--- .github/workflows/release-go-task.yml | 10 +++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/.github/workflows/publish-go-nightly-task.yml b/.github/workflows/publish-go-nightly-task.yml index 6996e171..4b937a55 100644 --- a/.github/workflows/publish-go-nightly-task.yml +++ b/.github/workflows/publish-go-nightly-task.yml @@ -103,10 +103,14 @@ jobs: chmod +x "${{ env.DIST_DIR }}/${{ env.PROJECT_NAME }}_osx_darwin_amd64/${{ env.PROJECT_NAME }}" PACKAGE_FILENAME="$(basename ${{ env.DIST_DIR }}/${{ env.PROJECT_NAME }}_nightly-*_macOS_64bit.tar.gz)" tar -czvf "${{ env.DIST_DIR }}/$PACKAGE_FILENAME" \ - -C "${{ env.DIST_DIR }}/${{ env.PROJECT_NAME }}_osx_darwin_amd64/" "${{ env.PROJECT_NAME }}" \ - -C ../../ LICENSE.txt + -C "${{ env.DIST_DIR }}/${{ env.PROJECT_NAME }}_osx_darwin_amd64/" "${{ env.PROJECT_NAME }}" \ + -C ../../ LICENSE.txt CHECKSUM="$(shasum -a 256 ${{ env.DIST_DIR }}/$PACKAGE_FILENAME | cut -d " " -f 1)" - perl -pi -w -e "s/.*${PACKAGE_FILENAME}/${CHECKSUM} ${PACKAGE_FILENAME}/g;" ${{ env.DIST_DIR }}/*-checksums.txt + perl \ + -pi \ + -w \ + -e "s/.*${PACKAGE_FILENAME}/${CHECKSUM} ${PACKAGE_FILENAME}/g;" \ + ${{ env.DIST_DIR }}/*-checksums.txt - name: Upload artifacts uses: actions/upload-artifact@v2 diff --git a/.github/workflows/release-go-task.yml b/.github/workflows/release-go-task.yml index 193d22e3..31d1da9b 100644 --- a/.github/workflows/release-go-task.yml +++ b/.github/workflows/release-go-task.yml @@ -108,10 +108,14 @@ jobs: chmod +x ${{ env.DIST_DIR }}/${{ env.PROJECT_NAME }}_osx_darwin_amd64/${{ env.PROJECT_NAME }} TAG="${GITHUB_REF/refs\/tags\//}" tar -czvf "${{ env.DIST_DIR }}/${{ env.PROJECT_NAME }}_${TAG}_macOS_64bit.tar.gz" \ - -C ${{ env.DIST_DIR }}/${{ env.PROJECT_NAME }}_osx_darwin_amd64/ ${{ env.PROJECT_NAME }} \ - -C ../../ LICENSE.txt + -C ${{ env.DIST_DIR }}/${{ env.PROJECT_NAME }}_osx_darwin_amd64/ ${{ env.PROJECT_NAME }} \ + -C ../../ LICENSE.txt CHECKSUM="$(shasum -a 256 ${{ env.DIST_DIR }}/${{ env.PROJECT_NAME }}_${TAG}_macOS_64bit.tar.gz | cut -d " " -f 1)" - perl -pi -w -e "s/.*${{ env.PROJECT_NAME }}_${TAG}_macOS_64bit.tar.gz/${CHECKSUM} ${{ env.PROJECT_NAME }}_${TAG}_macOS_64bit.tar.gz/g;" ${{ env.DIST_DIR }}/*-checksums.txt + perl \ + -pi \ + -w \ + -e "s/.*${{ env.PROJECT_NAME }}_${TAG}_macOS_64bit.tar.gz/${CHECKSUM} ${{ env.PROJECT_NAME }}_${TAG}_macOS_64bit.tar.gz/g;" \ + ${{ env.DIST_DIR }}/*-checksums.txt - name: Upload artifacts uses: actions/upload-artifact@v2 From 84aeda6ea4091947d644f2d773bdb619b698b86e Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 2 Sep 2021 00:15:42 -0700 Subject: [PATCH 5/9] Use shallow fetch for Go tester build The default behavior of the `actions/checkout` action is to do a shallow fetch of the repository, which is the most efficient if all that's needed is a copy of the repository files. In cases where the repository history is needed, this behavior is not appropriate, and so it can be configured via the `fetch-depth` input. Setting this input to 0 causes the full history to be fetched. A full fetch is required for workflows that use the `arduino/create-changelog` action to generate a raw changelog from the commit history, and so it is found in the "Release" workflow. However, there is no use of the commit history by the "Publish Tester Build" workflow, which does not need a changelog. The unnecessary full fetch makes the workflow less efficient and more difficult to understand, so it must be removed. --- .github/workflows/publish-go-tester-task.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/publish-go-tester-task.yml b/.github/workflows/publish-go-tester-task.yml index ab9b83cf..94cd19ae 100644 --- a/.github/workflows/publish-go-tester-task.yml +++ b/.github/workflows/publish-go-tester-task.yml @@ -60,8 +60,6 @@ jobs: steps: - name: Checkout repository uses: actions/checkout@v2 - with: - fetch-depth: 0 - name: Install Task uses: arduino/setup-task@v1 From 105d08ff5dcc7e2efaf50e5a9837fafc05df9aee Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 2 Sep 2021 00:16:30 -0700 Subject: [PATCH 6/9] Remove superfluous checkout step from "Release" workflow The `create-release` job of the "Release" workflow creates a GitHub release and uploads the built binaries as release assets. Since the binaries come from the workflow artifact, there is no need for this job to checkout the repository. The pre-release identification step uses the `GITHUB_REF` environment variable defined by GitHub Actions, which is not dependent on the runner having a repository checked out. This means the checkout step serves no purpose. Since it makes the workflow less efficient and more difficult to maintain, it must be removed. --- .github/workflows/release-go-task.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/release-go-task.yml b/.github/workflows/release-go-task.yml index 31d1da9b..0b580dad 100644 --- a/.github/workflows/release-go-task.yml +++ b/.github/workflows/release-go-task.yml @@ -129,9 +129,6 @@ jobs: needs: notarize-macos steps: - - name: Checkout repository - uses: actions/checkout@v2 - - name: Download artifact uses: actions/download-artifact@v2 with: From 7ab3a1705ff117edb4a689752ce6f0fd98746c84 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 2 Sep 2021 00:16:56 -0700 Subject: [PATCH 7/9] Add explanatory comment re: release asset upload warning The subfolders that remain in the Go release system's build output cause a bunch of warnings in the workflow run summary and logs resulting from the step that uploads the archives as release assets: ``` Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Linux_32bit/. Directories can not be uploaded to a release. Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Linux_64bit/. Directories can not be uploaded to a release. Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Linux_ARM64/. Directories can not be uploaded to a release. Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Linux_ARMv6/. Directories can not be uploaded to a release. Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Linux_ARMv7/. Directories can not be uploaded to a release. Warning: Artifact is a directory:dist/serial-discovery_0.0.42_macOS_64bit/. Directories can not be uploaded to a release. Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Windows_32bit/. Directories can not be uploaded to a release. Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Windows_64bit/. Directories can not be uploaded to a release. ``` There is no problem because these subfolders are not intended to be added as release assets and the archives in the root of the dist folder are uploaded, but I think these warnings might cause someone confusion far in the future when nobody remembers exactly how these workflows work and wonders if these warnings indicate something is wrong. I didn't find a clean solution for adjusting the artifacts input glob to exclude the subfolders so I settled on adding an explanatory comment to the workflow. --- .github/workflows/release-go-task.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/release-go-task.yml b/.github/workflows/release-go-task.yml index 0b580dad..2423dba3 100644 --- a/.github/workflows/release-go-task.yml +++ b/.github/workflows/release-go-task.yml @@ -151,6 +151,8 @@ jobs: bodyFile: ${{ env.DIST_DIR }}/CHANGELOG.md draft: false prerelease: ${{ steps.prerelease.outputs.IS_PRE }} + # NOTE: "Artifact is a directory" warnings are expected and don't indicate a problem + # (all the files we need are in the DIST_DIR root) artifacts: ${{ env.DIST_DIR }}/* - name: Upload release files on Arduino downloads servers From c679381898d271f8d2ba138a03a4ae36b3c55548 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 2 Sep 2021 00:17:55 -0700 Subject: [PATCH 8/9] Add infrastructure for defining -ldflags for go test run The `go:build` task has an `LDFLAGS` taskfile template variable, which is currently used by some projects to inject the versioning information while building via an `-ldflags` flag. `go test` has undocumented support for an `-ldflags` flag as well. Projects might find it useful to be able to set the value of string variables in the code while running the tests. To provide support by the task for this use case, a `TEST_LDFLAGS` taskfile template variable is added to the `go test` command in the `go:test` task, as well as empty definition of the variable. Even though this project doesn't currently have any need for the capability, it does no harm and brings the task into alignment with the standardized "template". --- Taskfile.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Taskfile.yml b/Taskfile.yml index 620d9941..b9a42063 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -39,6 +39,8 @@ vars: -X {{.CONFIGURATION_PACKAGE}}.Commit={{.COMMIT}} -X {{.CONFIGURATION_PACKAGE}}.Timestamp={{.TIMESTAMP}} ' + # `-ldflags` flag to use for `go test` command + TEST_LDFLAGS: tasks: build: @@ -225,6 +227,7 @@ tasks: -run '{{default ".*" .GO_TEST_REGEX}}' \ {{default "-timeout 10m -coverpkg=./... -covermode=atomic" .GO_TEST_FLAGS}} \ -coverprofile=coverage_unit.txt \ + {{.TEST_LDFLAGS}} \ {{default .DEFAULT_GO_PACKAGES .GO_PACKAGES}} # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/test-go-integration-task/Taskfile.yml From 0ba765766c00152aa413665be0c9207f8e190ee0 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 2 Sep 2021 00:20:29 -0700 Subject: [PATCH 9/9] Run formatting task after documentation generation The tasks under the `deps` key run in parallel, which is not appropriate for the task that is intended to format the output from the documentation generation tasks. The generation tasks can run concurrently, so they stay in `deps`, but the formatting task must be placed in `cmds`, which will cause it to run after all tasks in `deps` have finished. --- Taskfile.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/Taskfile.yml b/Taskfile.yml index b9a42063..f9334bd6 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -109,6 +109,7 @@ tasks: deps: - task: go:cli-docs - task: go:rule-docs + cmds: # Make the formatting consistent with the non-generated Markdown - task: general:format-prettier