Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

travis: remove go1.12 job #1784

Merged
merged 1 commit into from
Sep 20, 2019
Merged

travis: remove go1.12 job #1784

merged 1 commit into from
Sep 20, 2019

Conversation

skylenet
Copy link
Contributor

This is not needed. The Ubuntu PPA is using go1.11.

@@ -21,19 +21,6 @@ jobs:
script:
- go run build/ci.go lint

# Go 1.12.x is needed because of the Ubuntu PPA builds
Copy link
Member

Choose a reason for hiding this comment

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

Go 1.12 is not needed for PPA, but it is a supported Go version. Last two releases are. Maybe we should keep it for this reason and change the 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.

Why do we need to support go1.12?

Copy link
Member

Choose a reason for hiding this comment

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

I thought that we supported current and previous go version. I could be wrong. If we are supporting only the current, then the job is not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously we were supporting 1.11 and 1.12 mainly because the Ubuntu PPA needs 1.11.

IMO we can remove 1.12 because none of our build systems need it. Our binaries are published using 1.13.

I'm also in favor of getting rid of 1.11 and the Ubuntu PPA builds, but that's a topic for the roundtable :)

Copy link
Member

Choose a reason for hiding this comment

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

Maybe our codebase does not need to ensure support for previous go version for people that cannot upgrade right away, as it is easy to have two version one beside another. In that case, removing this job is ok.

Copy link
Member

Choose a reason for hiding this comment

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

i also thought it was deliberate check for backward compatibility one version back. I would leave it. is it really making CI faster given parallelism?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Backward compatibility is not needed IMO. Our binaries are released with go1.13, so why should we care about supporting go1.12?

Copy link
Member

Choose a reason for hiding this comment

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

Given that this is an open source project, someone may still use older but still supported Go version to build or use some of the packages from swarm. Maybe we still did not reach that level of project maturity or popularity, so ensuring that project works only with the latest release is just fine, and that we can think about this later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just a comment on the CI from my side - we have 5 concurrent jobs in total for the ethersphere organisation, so if half of them are used on jobs we don't need, CI is essentially twice slower, specially when you have to wait for free executors - yesterday we waited for one-two hours to get helm charts released, due to many PRs being submitted at the same time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we still did not reach that level of project maturity or popularity, so ensuring that project works only with the latest release is just fine, and that we can think about this later. - exactly my opinion on this PR.

@skylenet skylenet merged commit f37e979 into master Sep 20, 2019
@skylenet
Copy link
Contributor Author

Thanks for the reviews and valid opinions. I'll merge it for now. We can still add it back If we ever have a case where someone need this to be compatible with another go version.

chadsr added a commit to chadsr/swarm that referenced this pull request Sep 23, 2019
* 'master' of github.com:ethersphere/swarm: (32 commits)
  network/stream: refactor cursors tests (ethersphere#1786)
  network: Add capabilities if peer from store does not have it (ethersphere#1791)
  Swap logger (ethersphere#1754)
  network: Add capability filtered depth calculation (ethersphere#1787)
  travis: remove go1.12 job (ethersphere#1784)
  cmd/swarm: correct bzznetworkid flag description (ethersphere#1761)
  network, pss: Capability in pss (ethersphere#1764)
  network/stream: handle nil peer in TestNodesExchangeCorrectBinIndexes (ethersphere#1779)
  protocols, retrieval: swap-enabled messages implement Price (ethersphere#1771)
  cmd/swarm-smoke: fix waitToPushSynced connection closing (ethersphere#1781)
  cmd/swarm: simplify testCluster.StartNewNodes (ethersphere#1777)
  build: increase golangci-lint deadline (ethersphere#1778)
  docker: ignore build/bin when copying files (ethersphere#1780)
  swap: fix and rename Peer.getLastSentCumulativePayout (ethersphere#1769)
  network/stream: more resilient TestNodesCorrectBinsDynamic (ethersphere#1776)
  network: Add Capabilities to Kademlia database (ethersphere#1713)
  network: add own address to KademliaInfo (ethersphere#1775)
  pss: Refactor. Step 2. Refactor forward cache (ethersphere#1742)
  all: configurable payment/disconnect thresholds (ethersphere#1729)
  network/stream/v2: more resilient TestNodesExchangeCorrectBinIndexes (ethersphere#1760)
  ...
@skylenet skylenet added this to the 0.5.0 milestone Sep 25, 2019
@skylenet skylenet deleted the remove-go-1-12 branch September 30, 2019 11:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants