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

Add _lcov_merger attribute to test rules #1388

Merged
merged 2 commits into from
Jul 7, 2022

Conversation

keith
Copy link
Member

@keith keith commented Mar 12, 2022

As of bazel 5.1 this can be used as a late bound attribute bazelbuild/bazel@14d8be0

This alleviates the concerns from
#691 and allows users to
use this feature, and avoid having to pass LCOV_MERGER in the
environment manually

@lyft-lint-bot
Copy link

Lyft integration job started: https://buildkite.com/lyft/rules-apple/builds/444 (must be Lyft employee to view)

As of bazel 5.1 this can be used as a late bound attribute bazelbuild/bazel@14d8be0

This alleviates the concerns from
#691 and allows users to
use this feature, and avoid having to pass `LCOV_MERGER` in the
environment manually
@keith keith force-pushed the ks/add-_lcov_merger-attribute-to-test-rules branch from 7749a20 to 91ebb94 Compare April 16, 2022 03:32
@lyft-lint-bot
Copy link

Lyft integration job started: https://buildkite.com/lyft/rules-apple/builds/486 (must be Lyft employee to view)

@lyft-lint-bot
Copy link

Lyft integration job started: https://buildkite.com/lyft/rules-apple/builds/633 (must be Lyft employee to view)

@brentleyjones
Copy link
Collaborator

I would cut one more release before merging this, but then I think it's 👍

@keith
Copy link
Member Author

keith commented Jun 9, 2022

Do we need a new release first even? since it's in bazel 5.1+? I think of us supporting LTS meaning the newest in the LTS release version

@keith keith marked this pull request as ready for review June 9, 2022 17:47
@brentleyjones
Copy link
Collaborator

My thoughts are:

  • We have over 80 commits since the last release
  • This is a breaking change for some people, who can't go to 5.1 for some reason
  • So why not at least give them the latest changes as a release? We can even do two releases back to back if we want (maybe the second one with this has the new versioning we've been thinking about)?

It doesn't directly impact me, since I think anyone on 5.x should be on 5.2 right now, but that was my reasoning at least.

@keith
Copy link
Member Author

keith commented Jul 7, 2022

ok since we've done a release since then we can probably merge this one now?

@keith keith merged commit a464b59 into master Jul 7, 2022
@keith keith deleted the ks/add-_lcov_merger-attribute-to-test-rules branch July 7, 2022 19:12
keith added a commit that referenced this pull request Jul 8, 2022
We now set the related attribute instead #1388
@keith keith mentioned this pull request Jul 8, 2022
keith added a commit that referenced this pull request Jul 8, 2022
We now set the related attribute instead #1388
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