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

feat: fix-visibility release artifacts #67

Merged
merged 4 commits into from
Nov 17, 2021
Merged

Conversation

f0rmiga
Copy link
Contributor

@f0rmiga f0rmiga commented Nov 17, 2021

  • Refactors the release process to make it easier to include more artifacts.
  • Adds the fix-visibility plugin to the list of released artifacts.

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
@@ -16,18 +16,18 @@ jobs:
run: git status --porcelain
- name: bazel test //...
env:
# Bazelisk will download bazel to here
# Bazelisk will download bazel to here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we don't actually have to download bazelisk!
bazel-contrib/rules_bazel_integration_test#1 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will defer this to another PR.

XDG_CACHE_HOME: ~/.cache/bazel-repo
run:
run: |
# Test the repository.
bazel --bazelrc=.github/workflows/ci.bazelrc --bazelrc=.bazelrc test
--config=release //...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe we don't want to test with --config=release since that causes some cache misses due to stamping non-determinism?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could argue that we don't need a test at all since we will only tag commits on main that were already tested.

# shellcheck disable=SC2016
echo 'mkdir -p "${dst}"'

for artifact in "$@"; do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh, don't need $BUILD_WORKSPACE_DIRECTORY to hop back to source tree? I guess this binary has no runfiles so a different working dir

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the files are passed from $(locations ...) MAKEVARS from Bazel.


load("@io_bazel_rules_go//go:def.bzl", "go_binary")

PLATFORMS = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rad

@f0rmiga f0rmiga merged commit 0a37991 into main Nov 17, 2021
@f0rmiga f0rmiga deleted the plugin-release-artifacts branch November 17, 2021 18:23
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.

2 participants