Skip to content

Bump v1.1.0 compatibility test to v1.2.1 #883

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

Merged
merged 7 commits into from
Nov 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,19 @@ jobs:
- <<: *linux
env: TEST_SCRIPT=test_rules_scala
- <<: *linux
env: TEST_SCRIPT=test_rules_scala BAZEL_VERSION=1.1.0
env: TEST_SCRIPT=test_rules_scala BAZEL_VERSION=1.2.1
- <<: *linux
env: TEST_SCRIPT=test_reproducibility
- <<: *linux
env: TEST_SCRIPT=test_reproducibility BAZEL_VERSION=1.1.0
env: TEST_SCRIPT=test_reproducibility BAZEL_VERSION=1.2.1
- <<: *osx
env: TEST_SCRIPT=test_rules_scala
- <<: *osx
env: TEST_SCRIPT=test_rules_scala BAZEL_VERSION=1.1.0
env: TEST_SCRIPT=test_rules_scala BAZEL_VERSION=1.2.1
- <<: *osx
env: TEST_SCRIPT=test_reproducibility
- <<: *osx
env: TEST_SCRIPT=test_reproducibility BAZEL_VERSION=1.1.0
env: TEST_SCRIPT=test_reproducibility BAZEL_VERSION=1.2.1

before_install:
- |
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ for an example workspace using another scala version.

| bazel | rules_scala gitsha |
|--------|--------------------|
| 1.1.0 | HEAD |
| 1.2.1 | HEAD |
| 0.28.0 | HEAD |
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we are no longer testing with an older version (we used to test with the oldest and the newest that worked and hoped all the ones in the middle would work).

We should probably find the sha that we switched away from 0.28.0 and put the label here or remove this table.

Since we don't test the oldest that works we don't really know what versions work for which shas, so if we are just going to test with 1, we should probably remove the table (since it isn't very accurate).

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 am fine either way as I am always using the latest though.

Copy link
Contributor Author

@chenrui333 chenrui333 Nov 26, 2019

Choose a reason for hiding this comment

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

Another thing I can think about this is to introduce more testing in the bazelci and it would cover more bazel versions.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's hard for many orgs to update large builds with many rules not all of which support the latest versions. It is much more of a service to tell users the versions that work for each version of bazel rather than them having to git-bisect it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, that is why I think this sha version table might be still quite useful though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will revert my 0.28.0 HEAD change.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to explicitly labeling the default commit.
@chenrui333 Id love for you to do this here but you can feel free to do it in a separate PR or not at all and I’ll do it.
Thanks!

Copy link
Contributor Author

@chenrui333 chenrui333 Nov 27, 2019

Choose a reason for hiding this comment

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

+1 to explicitly labeling the default commit.

@ittaiz I guess I missed on the default commit, do you prefer HEAD or you prefer explicit SHA (which is current HEAD)?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is great. I miswrote. I meant labeling in the Travis config and the test script the default Bazel version (not commit).
I’ll merge this as is.
If you can follow up with a PR with this I’d be grateful :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I can file a follow up PR on this.

| 0.23.x | ca655e5a330cbf1d66ce1d9baa63522752ec6011 | |
| 0.22.x | f3113fb6e9e35cb8f441d2305542026d98afc0a2 |
Expand Down
6 changes: 3 additions & 3 deletions tools/bazel
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ case "$bazel_version" in
darwin_sha='5d50ae13ba01a224ddf54cfd818289bee5b38e551cca22bffc79b89f377d2095'
linux_sha='a2a7e4cb38d7bc774a5bbfab4d45293f5f2158eb6caf0128792cc1148732a4e6'
;;
'1.1.0')
darwin_sha='1a552f4ce194860fbbd50eeb319f81788ddf50a849e92378eec72231cc64ef65'
linux_sha='14301099c87568db302d59a5d3585f5eb8a6250ac2c6bb0367c56e623ff6e65f'
'1.2.1')
darwin_sha='59e469bf1d8d1615b67856ea17e761be05e9c92b462e55c0354cd78145b480d5'
linux_sha='ae6249e25b0f5a06d79fad90325477dd56275657951cf7aa6a3cbcd79fc4d749'
;;
*)
echo "The requested Bazel version '$bazel_version' is not supported"
Expand Down